From 1f991db72aa2fcc941efa9076e68a0faed1fad50 Mon Sep 17 00:00:00 2001
From: Nico Alt <nicoalt@posteo.org>
Date: Thu, 6 Jun 2019 22:06:17 +0200
Subject: [PATCH] Add tests with pylint and pycodestyle

---
 .gitlab-ci.yml                          |   7 ++
 .pylintrc                               |   5 ++
 requirements-dev.txt                    |   3 +
 requirements.txt                        |   2 +
 src/briar-gtk.in                        |   3 +
 src/briar/api/api.py                    |  14 +--
 src/briar/api/model.py                  |   2 +-
 src/briar/api/models/contacts.py        |  14 +--
 src/briar/api/models/private_chat.py    |   7 +-
 src/briar/api/models/socket_listener.py |  16 ++--
 src/briar/gtk/application.py            |  25 +++---
 src/briar/gtk/container.py              |   2 -
 src/briar/gtk/containers/chat.py        |   7 +-
 src/briar/gtk/containers/main.py        |  14 +--
 src/briar/gtk/containers/startup.py     |  17 ++--
 src/briar/gtk/define.py                 |   2 +-
 src/briar/gtk/toolbar.py                |   3 -
 src/briar/gtk/window.py                 | 112 ++++++++++++------------
 src/meson_post_install.py               |  18 ++--
 tools/run-tests.sh                      |   4 +
 tools/tests/test-pycodestyle.sh         |   3 +
 tools/tests/test-pylint.sh              |   3 +
 22 files changed, 161 insertions(+), 122 deletions(-)
 create mode 100644 .gitlab-ci.yml
 create mode 100644 .pylintrc
 create mode 100644 requirements-dev.txt
 create mode 100644 requirements.txt
 create mode 100755 tools/run-tests.sh
 create mode 100755 tools/tests/test-pycodestyle.sh
 create mode 100755 tools/tests/test-pylint.sh

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 0000000..41c9760
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,7 @@
+test:lints:
+    image: debian:stretch
+    script:
+        - apt update && apt install --no-install-recommends -y gir1.2-gtk-3.0 python3-gi python3-pip python3-setuptools
+        - pip3 install -r requirements-dev.txt
+        - tools/tests/test-pycodestyle.sh
+        - tools/tests/test-pylint.sh
diff --git a/.pylintrc b/.pylintrc
new file mode 100644
index 0000000..bf12f55
--- /dev/null
+++ b/.pylintrc
@@ -0,0 +1,5 @@
+[MASTER]
+
+disable=attribute-defined-outside-init,
+	fixme,
+        missing-docstring
diff --git a/requirements-dev.txt b/requirements-dev.txt
new file mode 100644
index 0000000..1180199
--- /dev/null
+++ b/requirements-dev.txt
@@ -0,0 +1,3 @@
+-r requirements.txt
+pycodestyle>=2.5.0
+pylint>=2.3.1
diff --git a/requirements.txt b/requirements.txt
new file mode 100644
index 0000000..12fa859
--- /dev/null
+++ b/requirements.txt
@@ -0,0 +1,2 @@
+requests2>=2.16
+websockets>=7.0
diff --git a/src/briar-gtk.in b/src/briar-gtk.in
index e96a564..1f3930d 100755
--- a/src/briar-gtk.in
+++ b/src/briar-gtk.in
@@ -13,6 +13,9 @@ import os
 import signal
 import sys
 
+import gi
+gi.require_version('Gtk', '3.0')
+
 pkgdatadir = '@pkgdatadir@'
 localedir = '@localedir@'
 
diff --git a/src/briar/api/api.py b/src/briar/api/api.py
index 1a8a99a..cf83576 100644
--- a/src/briar/api/api.py
+++ b/src/briar/api/api.py
@@ -2,9 +2,6 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 # License-Filename: LICENSE.md
 
-from briar.api.constants import BASE_HTTP_URL, BRIAR_AUTH_TOKEN, BRIAR_DB
-from briar.api.models.socket_listener import SocketListener
-
 from os.path import isfile
 from subprocess import Popen, PIPE, STDOUT
 from threading import Thread
@@ -12,6 +9,9 @@ from time import sleep
 from urllib.error import HTTPError, URLError
 from urllib.request import urlopen
 
+from briar.api.constants import BASE_HTTP_URL, BRIAR_AUTH_TOKEN, BRIAR_DB
+from briar.api.models.socket_listener import SocketListener
+
 
 class Api:
 
@@ -25,7 +25,8 @@ class Api:
 
         self.socket_listener = SocketListener(self)
 
-    def has_account(self):
+    @staticmethod
+    def has_account():
         return isfile(BRIAR_DB)
 
     def is_running(self):
@@ -65,7 +66,7 @@ class Api:
                 urlopen(BASE_HTTP_URL)
                 sleep(0.1)
             except HTTPError as http_error:
-                if(http_error.code == 404):
+                if http_error.code == 404:
                     self._load_auth_token()
                     callback(True)
                     return
@@ -87,8 +88,7 @@ class Api:
                                    credentials[1] + '\n').encode("utf-8"))
 
     def _load_auth_token(self):
-        if not self.has_account():
+        if not Api.has_account():
             raise Exception("Can't load authentication token")
         with open(BRIAR_AUTH_TOKEN, 'r') as file:
             self.auth_token = file.read()
-
diff --git a/src/briar/api/model.py b/src/briar/api/model.py
index e28cb2d..3a8fdaa 100644
--- a/src/briar/api/model.py
+++ b/src/briar/api/model.py
@@ -3,7 +3,7 @@
 # License-Filename: LICENSE.md
 
 
-class Model:
+class Model:  # pylint: disable=too-few-public-methods
 
     _headers = {}
 
diff --git a/src/briar/api/models/contacts.py b/src/briar/api/models/contacts.py
index 85c57e8..a633c24 100644
--- a/src/briar/api/models/contacts.py
+++ b/src/briar/api/models/contacts.py
@@ -2,16 +2,18 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 # License-Filename: LICENSE.md
 
-from briar.api.constants import BASE_HTTP_URL
-from briar.api.model import Model
+from urllib.parse import urljoin
 
 from requests import get as _get
-from urllib.parse import urljoin
+
+from briar.api.constants import BASE_HTTP_URL
+from briar.api.model import Model
 
 
-class Contacts(Model):
+# TODO: remove pylint disable once we have more methods
+class Contacts(Model):  # pylint: disable=too-few-public-methods
 
     def get(self):
         url = urljoin(BASE_HTTP_URL, 'contacts')
-        r = _get(url, headers=self._headers)
-        return r.json()
+        request = _get(url, headers=self._headers)
+        return request.json()
diff --git a/src/briar/api/models/private_chat.py b/src/briar/api/models/private_chat.py
index 6255526..abf5e3b 100644
--- a/src/briar/api/models/private_chat.py
+++ b/src/briar/api/models/private_chat.py
@@ -2,12 +2,13 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 # License-Filename: LICENSE.md
 
-from briar.api.constants import BASE_HTTP_URL
-from briar.api.model import Model
+from urllib.parse import urljoin
 
 from requests import get as _get
 from requests import post as _post
-from urllib.parse import urljoin
+
+from briar.api.constants import BASE_HTTP_URL
+from briar.api.model import Model
 
 
 class PrivateChat(Model):
diff --git a/src/briar/api/models/socket_listener.py b/src/briar/api/models/socket_listener.py
index 1f429dc..93808a0 100644
--- a/src/briar/api/models/socket_listener.py
+++ b/src/briar/api/models/socket_listener.py
@@ -2,17 +2,19 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 # License-Filename: LICENSE.md
 
-from briar.api.constants import WEBSOCKET_URL
-from briar.api.model import Model
-
 import asyncio
 import json
 from threading import Thread
+
 import websockets
 
+from briar.api.constants import WEBSOCKET_URL
+from briar.api.model import Model
+
 
 # TODO: Make more general; currently very specific to private messages
-class SocketListener(Model):
+# TODO: remove pylint disable once we have more methods
+class SocketListener(Model):  # pylint: disable=too-few-public-methods
 
     def watch(self, callback, event, contact_id="0"):
         websocket_thread = Thread(target=self._start_watch_loop,
@@ -27,6 +29,8 @@ class SocketListener(Model):
         loop.run_forever()
         loop.close()
 
+    # TODO: use contact id
+    # pylint: disable=unused-argument
     async def _start_websocket(self, callback, event, contact_id="0"):
         async with websockets.connect(WEBSOCKET_URL) as websocket:
             await websocket.send(self._api.auth_token)
@@ -34,11 +38,11 @@ class SocketListener(Model):
 
     async def _watch_messages(self, websocket, event, callback):
         while not websocket.closed and not\
-                         asyncio.get_event_loop().is_closed():
+                asyncio.get_event_loop().is_closed():
             message = await websocket.recv()
             message = json.loads(message)
             if message['name'] == event:
                 callback(message['data'])
         if not asyncio.get_event_loop().is_closed():
             asyncio.get_event_loop().create_task(
-                self._watch_messages(websocket, callback))
+                self._watch_messages(websocket, event, callback))
diff --git a/src/briar/gtk/application.py b/src/briar/gtk/application.py
index 26349e9..ca25402 100644
--- a/src/briar/gtk/application.py
+++ b/src/briar/gtk/application.py
@@ -6,14 +6,12 @@
 # Initial version based on GNOME Lollypop
 # https://gitlab.gnome.org/World/lollypop/blob/1.0.2/lollypop/application.py
 
+from gi.repository import Gdk, Gio, GLib, Gtk
+
 from briar.api.api import Api
 
 from briar.gtk.window import Window
 
-import gi
-gi.require_version('Gtk', '3.0')
-from gi.repository import Gdk, Gio, GLib, Gtk
-
 
 class Application(Gtk.Application):
 
@@ -22,28 +20,35 @@ class Application(Gtk.Application):
         GLib.set_prgname("Briar")
         super().__init__(application_id='app.briar.gtk')
         self.window = None
+        self._set_version(version)
 
+    # pylint: disable=arguments-differ
     def do_startup(self):
         Gtk.Application.do_startup(self)
 
-        cssProviderFile = Gio.File.new_for_uri(
+        css_provider_file = Gio.File.new_for_uri(
             "resource:///app/briar/gtk/ui/application.css")
-        cssProvider = Gtk.CssProvider()
-        cssProvider.load_from_file(cssProviderFile)
+        css_provider = Gtk.CssProvider()
+        css_provider.load_from_file(css_provider_file)
         screen = Gdk.Screen.get_default()
-        styleContext = Gtk.StyleContext()
-        styleContext.add_provider_for_screen(screen, cssProvider,
-                                             Gtk.STYLE_PROVIDER_PRIORITY_USER)
+        style_context = Gtk.StyleContext()
+        style_context.add_provider_for_screen(screen, css_provider,
+                                              Gtk.STYLE_PROVIDER_PRIORITY_USER)
 
         self.api = Api('/app/briar/briar-headless.jar')
 
+    # pylint: disable=arguments-differ
     def do_activate(self):
         if self.window is None:
             self.window = Window()
             self.window.show()
         self.window.present()
 
+    # pylint: disable=arguments-differ
     def quit(self):
         self.api.stop()
         self.window.hide()
         Gio.Application.quit(self)
+
+    def _set_version(self, version):
+        self._version = version
diff --git a/src/briar/gtk/container.py b/src/briar/gtk/container.py
index a54554c..94ce877 100644
--- a/src/briar/gtk/container.py
+++ b/src/briar/gtk/container.py
@@ -2,8 +2,6 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 # License-Filename: LICENSE.md
 
-import gi
-gi.require_version('Gtk', '3.0')
 from gi.repository import Gtk
 
 
diff --git a/src/briar/gtk/containers/chat.py b/src/briar/gtk/containers/chat.py
index bf98ff5..3ffd81e 100644
--- a/src/briar/gtk/containers/chat.py
+++ b/src/briar/gtk/containers/chat.py
@@ -2,14 +2,12 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 # License-Filename: LICENSE.md
 
+from gi.repository import GLib, Gtk
+
 from briar.api.models.private_chat import PrivateChat
 from briar.gtk.container import Container
 from briar.gtk.define import App
 
-import gi
-gi.require_version('Gtk', '3.0')
-from gi.repository import GLib, Gtk
-
 
 class ChatContainer(Container):
 
@@ -46,6 +44,7 @@ class ChatContainer(Container):
     def _add_message_async(self, message):
         GLib.idle_add(self._add_message, message["text"], False)
 
+    # pylint: disable=unused-argument
     def _key_pressed(self, widget, event):
         if event.hardware_keycode != 36 and event.hardware_keycode != 104:
             return
diff --git a/src/briar/gtk/containers/main.py b/src/briar/gtk/containers/main.py
index ae3987e..c317616 100644
--- a/src/briar/gtk/containers/main.py
+++ b/src/briar/gtk/containers/main.py
@@ -2,14 +2,12 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 # License-Filename: LICENSE.md
 
+from gi.repository import GLib, Gtk
+
 from briar.api.models.contacts import Contacts
 from briar.gtk.container import Container
 from briar.gtk.define import App
 
-import gi
-gi.require_version('Gtk', '3.0')
-from gi.repository import GLib, Gtk
-
 
 class MainContainer(Container):
 
@@ -30,10 +28,12 @@ class MainContainer(Container):
         contacts_list_box = self.builder.get_object("contacts_list")
         for contact in contacts_list:
             contact_button = Gtk.Button(contact["author"]["name"])
-            contact_button.connect("clicked", self._contact_clicked,
+            contact_button.connect("clicked", MainContainer._contact_clicked,
                                    contact["contactId"])
             contact_button.show()
             contacts_list_box.add(contact_button)
 
-    def _contact_clicked(self, widget, contactId):
-        GLib.idle_add(App().window.open_private_chat, contactId)
+    # pylint: disable=unused-argument
+    @staticmethod
+    def _contact_clicked(widget, contact_id):
+        GLib.idle_add(App().window.open_private_chat, contact_id)
diff --git a/src/briar/gtk/containers/startup.py b/src/briar/gtk/containers/startup.py
index 66930c9..f907798 100644
--- a/src/briar/gtk/containers/startup.py
+++ b/src/briar/gtk/containers/startup.py
@@ -2,39 +2,42 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 # License-Filename: LICENSE.md
 
+from gi.repository import GLib, GObject, Gtk
+
 from briar.gtk.container import Container
 from briar.gtk.define import App
 
-import gi
-gi.require_version('Gtk', '3.0')
-from gi.repository import GLib, GObject, Gtk
-
 
 class StartupContainer(Container):
 
     def __init__(self):
         super().__init__()
         self._api = App().api
-        self._register_signals()
+        StartupContainer._register_signals()
         self._setup_view()
 
+    # pylint: disable=unused-argument
     def on_username_button_clicked(self, button):
         self.builder.get_object("username_grid").set_visible(False)
         self.builder.get_object("password_grid").set_visible(True)
         self.username = self.builder.get_object("username_entry").get_text()
 
+    # pylint: disable=unused-argument
     def on_password_button_clicked(self, button):
         password = self.builder.get_object("password_entry").get_text()
-        password_confirm = self.builder.get_object("password_confirm_entry").get_text()
+        password_confirm = self.builder.get_object(
+            "password_confirm_entry").get_text()
         if password != password_confirm:
             raise Exception("Passwords do not match")
         self._api.register((self.username, password), self._startup_finished)
 
+    # pylint: disable=unused-argument
     def on_login_pressed(self, button):
         password = self.builder.get_object("password_entry").get_text()
         self._api.login(password, self._startup_finished)
 
-    def _register_signals(self):
+    @staticmethod
+    def _register_signals():
         GObject.signal_new("briar_startup_completed", Gtk.Overlay,
                            GObject.SignalFlags.RUN_LAST, GObject.TYPE_BOOLEAN,
                            (GObject.TYPE_STRING,))
diff --git a/src/briar/gtk/define.py b/src/briar/gtk/define.py
index c2c9aaf..66ec252 100644
--- a/src/briar/gtk/define.py
+++ b/src/briar/gtk/define.py
@@ -8,4 +8,4 @@
 
 from gi.repository import Gio
 
-App = Gio.Application.get_default
+App = Gio.Application.get_default  # pylint: disable=invalid-name
diff --git a/src/briar/gtk/toolbar.py b/src/briar/gtk/toolbar.py
index c2e167b..22fb2b0 100644
--- a/src/briar/gtk/toolbar.py
+++ b/src/briar/gtk/toolbar.py
@@ -6,8 +6,6 @@
 # Initial version based on GNOME Lollypop
 # https://gitlab.gnome.org/World/lollypop/blob/1.0.12/lollypop/toolbar.py
 
-import gi
-gi.require_version('Gtk', '3.0')
 from gi.repository import Gtk
 
 
@@ -30,4 +28,3 @@ class Toolbar(Gtk.HeaderBar):
             raise Exception("Callback needed when showing back button")
         back_button.show()
         back_button.connect("clicked", callback)
-
diff --git a/src/briar/gtk/window.py b/src/briar/gtk/window.py
index e2e3a46..99962c9 100644
--- a/src/briar/gtk/window.py
+++ b/src/briar/gtk/window.py
@@ -2,86 +2,86 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 # License-Filename: LICENSE.md
 
+from gi.repository import Gtk
+
 from briar.gtk.containers.chat import ChatContainer
 from briar.gtk.containers.main import MainContainer
 from briar.gtk.containers.startup import StartupContainer
 from briar.gtk.define import App
 from briar.gtk.toolbar import Toolbar
 
-import gi
-gi.require_version('Gtk', '3.0')
-from gi.repository import Gtk
-
 
 class Window(Gtk.ApplicationWindow):
 
     def __init__(self):
         Gtk.ApplicationWindow.__init__(self, application=App(), title="Briar",
                                        icon_name="app.briar.gtk")
-        self.__setup_content()
+        self._setup_content()
 
     @property
     def container(self):
-        return self.__container
+        return self._container
 
     @property
     def toolbar(self):
-        return self.__toolbar
+        return self._toolbar
 
-    def __setup_content(self):
-        self.__setup_size((600, 400))  # TODO: do properly (constants, save)
-        self.__setup_toolbar()
-        self.__setup_grid()
-        self.__setup_startup_container()
+    def _setup_content(self):
+        self._setup_size((600, 400))  # TODO: do properly (constants, save)
+        self._setup_toolbar()
+        self._setup_grid()
+        self._setup_startup_container()
 
-    def __setup_size(self, size):
+    def _setup_size(self, size):
         if len(size) == 2 and\
            isinstance(size[0], int) and\
            isinstance(size[1], int):
             self.resize(size[0], size[1])
 
-    def __setup_toolbar(self):
-        self.__toolbar = Toolbar()
-        self.__toolbar.show()
-        self.__toolbar.set_show_close_button(True)
-        self.set_titlebar(self.__toolbar)
-
-    def __setup_grid(self):
-        self.__grid = Gtk.Grid()
-        self.__grid.set_orientation(Gtk.Orientation.HORIZONTAL)
-        self.__grid.show()
-        self.add(self.__grid)
-
-    def __setup_startup_container(self):
-        self.__container = StartupContainer()
-        self.__container.show()
-        self.__container.connect("briar_startup_completed",
-                                 self.__on_startup_completed)
-        self.__grid.add(self.__container)
-
-    def __on_startup_completed(self, inst, obj):
-        self.__grid.destroy()
-        self.__setup_grid()
-        self.__setup_main_container()
-
-    def __setup_main_container(self):
-        self.__container = MainContainer()
-        self.__container.show()
-        self.__grid.add(self.__container)
+    def _setup_toolbar(self):
+        self._toolbar = Toolbar()
+        self._toolbar.show()
+        self._toolbar.set_show_close_button(True)
+        self.set_titlebar(self._toolbar)
+
+    def _setup_grid(self):
+        self._grid = Gtk.Grid()
+        self._grid.set_orientation(Gtk.Orientation.HORIZONTAL)
+        self._grid.show()
+        self.add(self._grid)
+
+    def _setup_startup_container(self):
+        self._container = StartupContainer()
+        self._container.show()
+        self._container.connect("briar_startup_completed",
+                                self._on_startup_completed)
+        self._grid.add(self._container)
+
+    # TODO: remove unused arguments
+    # pylint: disable=unused-argument
+    def _on_startup_completed(self, inst, obj):
+        self._grid.destroy()
+        self._setup_grid()
+        self._setup_main_container()
+
+    def _setup_main_container(self):
+        self._container = MainContainer()
+        self._container.show()
+        self._grid.add(self._container)
 
     def open_private_chat(self, contact_id):
-        self.__grid.destroy()
-        self.__setup_grid()
-        self.__setup_private_chat(contact_id)
-
-    def __setup_private_chat(self, contact_id):
-        self.__container = ChatContainer(contact_id)
-        self.__container.show()
-        self.__grid.add(self.__container)
-        self.__toolbar.show_back_button(True, self.__back_to_main)
-
-    def __back_to_main(self, widget):
-        self.__grid.destroy()
-        self.__setup_grid()
-        self.__toolbar.show_back_button(False)
-        self.__setup_main_container()
+        self._grid.destroy()
+        self._setup_grid()
+        self._setup_private_chat(contact_id)
+
+    def _setup_private_chat(self, contact_id):
+        self._container = ChatContainer(contact_id)
+        self._container.show()
+        self._grid.add(self._container)
+        self._toolbar.show_back_button(True, self._back_to_main)
+
+    def _back_to_main(self, widget):
+        self._grid.destroy()
+        self._setup_grid()
+        self._toolbar.show_back_button(False)
+        self._setup_main_container()
diff --git a/src/meson_post_install.py b/src/meson_post_install.py
index 367e2c4..1eae062 100755
--- a/src/meson_post_install.py
+++ b/src/meson_post_install.py
@@ -6,19 +6,19 @@
 from os import environ, path
 from subprocess import call
 
-prefix = environ.get('MESON_INSTALL_PREFIX', '/usr/local')
-datadir = path.join(prefix, 'share')
-destdir = environ.get('DESTDIR', '')
+PREFIX = environ.get('MESON_INSTALL_PREFIX', '/usr/local')
+DATA_DIR = path.join(PREFIX, 'share')
+DESTINATION_DIR = environ.get('DESTDIR', '')
 
 # Package managers set this so we don't need to run
-if not destdir:
+if not DESTINATION_DIR:
     print('Updating icon cache...')
-    call(['gtk-update-icon-cache', '-qtf', path.join(datadir, 'icons', 'hicolor')])
+    call(['gtk-update-icon-cache', '-qtf',
+          path.join(DATA_DIR, 'icons', 'hicolor')])
 
     print('Updating desktop database...')
-    call(['update-desktop-database', '-q', path.join(datadir, 'applications')])
+    call(['update-desktop-database', '-q',
+          path.join(DATA_DIR, 'applications')])
 
     print('Compiling GSettings schemas...')
-    call(['glib-compile-schemas', path.join(datadir, 'glib-2.0', 'schemas')])
-
-
+    call(['glib-compile-schemas', path.join(DATA_DIR, 'glib-2.0', 'schemas')])
diff --git a/tools/run-tests.sh b/tools/run-tests.sh
new file mode 100755
index 0000000..38df236
--- /dev/null
+++ b/tools/run-tests.sh
@@ -0,0 +1,4 @@
+#!/usr/bin/env bash
+
+tools/tests/test-pycodestyle.sh
+tools/tests/test-pylint.sh
diff --git a/tools/tests/test-pycodestyle.sh b/tools/tests/test-pycodestyle.sh
new file mode 100755
index 0000000..c334d1e
--- /dev/null
+++ b/tools/tests/test-pycodestyle.sh
@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+pycodestyle --show-source --show-pep8 src
diff --git a/tools/tests/test-pylint.sh b/tools/tests/test-pylint.sh
new file mode 100755
index 0000000..5f2b637
--- /dev/null
+++ b/tools/tests/test-pylint.sh
@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+pylint src
-- 
GitLab