Merge ~pieq/bugit/+git/qabro:fix-1887903-gi-snapd into bugit:master

[Pierre Equoy]

Summary
=======

This brings two different things:

1. A new way to discuss with Snapd, from using sockets to using gobject-introspection. This was suggested by jamesh 2 years ago[1] (!!!) and finally implemented, mostly to fix an annoying and hard to reproduce bug[2]

2. Some unit tests using pytest, and a contributing guidelines to help newcomers setting up test env and running unit tests.

I recommend reviewing the changes commit per commit for more clarity.

How it was tested
=================

1. Run the latest version of qabro on a device that exhibits bug lp:1887903 bug:

$ sudo snap install qabro --edge --devmode
qabro (edge) 0.14dev from Pierre Equoy (pieq) installed
$ qabro
Traceback (most recent call last):
  File "/snap/qabro/122/bin/qabro", line 33, in <module>
    sys.exit(load_entry_point('qabro==0.14.dev0', 'console_scripts', 'qabro')())
  (...)
  File "/snap/qabro/122/usr/lib/python3.6/json/decoder.py", line 357, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

2. Build a snap from this branch, and install it on the same device:

# locally
$ snapcraft
$ scp qabro_0.14dev_amd64.snap ...

# on the test device
$ sudo snap install qabro_0.14dev_amd64.snap --devmode

3. Run qabro again

$ qabro

This time, the interface shows up!

[1] https://forum.snapcraft.io/t/how-to-communicate-with-run-snapd-socket-using-python/6432/4
[2] https://bugs.launchpad.net/qabro/+bug/1887903

[Pierre Equoy (pieq)]

Following a discussion with Stanley regarding the use of pytest, I've revamped a bit the unit tests in order to parametrize some of them.

[Maciej Kisielewski (kissiel)]

I got some minor comments below. I really don't like the unit for checking the session path.

[Pierre Equoy (pieq)]

Thanks a lot for the feedback Maciek!

I commented below, I need your help for some clarification before I can resubmit.

[Maciej Kisielewski (kissiel)]

Comments below.

[Pierre Equoy (pieq)]

@Maciek: I've revamped the commits (and added one for the setup.py changes) with your suggestions. Let me know what you think. Thanks again!

[Maciej Kisielewski (kissiel)]

Thanks! Looks great, +1

review: Approve

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
new file mode 100644
index 0000000..d6266ab
--- /dev/null
+++ b/CONTRIBUTING.md
@@ -0,0 +1,113 @@
+Contributing
+============
+
+
+Setting up testing environment
+------------------------------
+
+If you want to run qabro without having to build a snap, you will need to setup
+a virtual environment:
+
+```
+# First, let's install packages required to build some of the dependencies
+$ sudo apt install libgirepository1.0-dev gcc libcairo2-dev pkg-config python3-dev gir1.2-gtk-3.0 gir1.2-snapd-1
+$ python3 -m venv venv
+$ source venv/bin/activate
+# From now on, you're inside the virtual env
+# Let's install the Python packages we need to run qabro and the unit tests
+(venv) $ pip install -e .[test]
+```
+
+
+Running the unit tests
+----------------------
+
+With a virtual environment set up:
+
+```
+$ python -m pytest
+```
+
+You can also run coverage to get a code coverage HTML report:
+
+```
+$ coverage run -m pytest
+```
+
+
+Running QABRO from within a virtual environment
+-----------------------------------------------
+
+Then, by using a python script such as this one (e.g. `test-qabro.py`):
+
+```
+#!/usr/bin/env python3
+import os
+import qabro
+
+os.environ['SNAP_NAME'] = 'qabro'
+os.environ['APPORT_LAUNCHPAD_INSTANCE'] = 'staging'
+
+qabro.main()
+```
+
+you should be able to run QABRO like this:
+
+```
+(venv) $ python test-qabro.py
+```
+
+Note that QABRO depends on several other things, some of them deeply tied to
+the snap environment, so if you want to test a whole run, it's better to build
+a test snap.
+
+
+Building a test snap
+--------------------
+
+Install snapcraft and multipass:
+
+```
+$ snap install snapcraft --classic
+$ multipass
+```
+
+Then, from within the qabro directory:
+
+```
+$ snapcraft
+Launching a VM.
+(...)
+Snapped qabro_0.14dev_amd64.snap
+```
+
+You can then install this snap onto your device or a test device, and run it:
+
+```
+$ snap install qabro_0.14dev_amd64.snap --devmode
+$ qabro
+```
+
+
+Submitting a change
+-------------------
+
+Set up a personal git remote for the project:
+
+```
+$ git remote add <myrepo> git+ssh://<username>@git.launchpad.net/~<username>/qabro/+git/qabro
+```
+
+Make your change on a dedicated branch:
+
+```
+$ git checkout -b my-new-branch
+# do your changes and commit them
+```
+
+Push the branch to your repo, then propose a merge request (Launchpad should
+indicate a URL to propose the merge request):
+
+```
+$ git push <myrepo>
+```
\ No newline at end of file
diff --git a/README b/README.md
similarity index 100%
rename from README
rename to README.md
diff --git a/qabro/__init__.py b/qabro/__init__.py
index 832e3a7..5604dcb 100644
--- a/qabro/__init__.py
+++ b/qabro/__init__.py
@@ -6,7 +6,7 @@ import sys
 
 from qabro.bug_assistant import BugAssistant, BugReport
 from qabro.ui import ReportScreen
-from qabro.utils import snapdsocket
+import qabro.utils
 
 
 logger = logging.getLogger(__name__)
@@ -21,10 +21,10 @@ def main():
         sys.exit(("Cannot determine snap name. "
                   "Please install this tool using the snap command."))
 
-    ss = snapdsocket()
-    qabro_snap_info = [e for e in ss.get_snaps() if e['name'] == 'qabro'][0]
+    snaps = qabro.utils.get_snaps()
+    qabro_snap_info = [e for e in snaps if e.props.name == 'qabro'][0]
 
-    if not qabro_snap_info['devmode']:
+    if not qabro_snap_info.props.devmode:
         sys.exit("This tool requires --devmode option to run properly.")
 
     logfile = os.path.join(os.getenv('SNAP_USER_COMMON', '/tmp'), 'qabro.log')
diff --git a/qabro/bug_assistant.py b/qabro/bug_assistant.py
index ed55c9d..6a720b0 100644
--- a/qabro/bug_assistant.py
+++ b/qabro/bug_assistant.py
@@ -10,7 +10,7 @@ from collections import Counter
 from launchpadlib.launchpad import Launchpad
 from launchpadlib.uris import LPNET_WEB_ROOT, STAGING_WEB_ROOT, QASTAGING_WEB_ROOT
 
-from qabro.utils import snapdsocket
+import qabro.utils
 
 
 class BugReport:
@@ -252,35 +252,35 @@ class AttachmentAssistant:
 
     def attach_snaplist(self):
         print("Retrieving list of installed snaps...")
-        sckt = snapdsocket()
-        snaps = sckt.get_snaps()
-        fmt = "{:<32}{:<16}{:<16}{:<16}{:<16}{:<16}"
+        snaps = qabro.utils.get_snaps()
+        fmt = "{:<32}{:<16}{:<16}{:<16}{:<16}{:<16}\n"
         out = fmt.format("Name", "Version", "Revision",
-                         "Channel", "Devmode", "Confinement") + "\n"
+                         "Channel", "Devmode", "Confinement")
         for snap in snaps:
-            if snap["devmode"]:
+            if snap.props.devmode:
                 devmode = "yes"
             else:
                 devmode = "no"
-            out += fmt.format(snap["name"], snap["version"], snap["revision"],
-                              snap.get("tracking-channel", "-"), devmode,
-                              snap["confinement"]) + "\n"
+            tracking_channel = snap.props.tracking_channel or "-"
+            out += fmt.format(snap.props.name, snap.props.version, snap.props.revision,
+                              tracking_channel, devmode,
+                              snap.props.confinement.value_nick)
         self.attachments["snap_list.log"] = out.encode()
 
-    def _get_last_checkbox_session_path(self):
+    def _get_last_checkbox_session_path(self, usr_dir="/home/$USER", root_dir="/root"):
         """Return path of the most recent Checkbox session directory."""
         rootpath = ''
         latest_session_dir = ''
         # Determine if we use a classic install or a snap install
-        classic_path = os.path.expandvars('/home/$USER/.cache/plainbox/sessions')
+        classic_path = os.path.expandvars(os.path.join(usr_dir, ".cache/plainbox/sessions"))
         if os.path.exists(classic_path):
             rootpath = classic_path
         else:
-            ss = snapdsocket()
-            snap_list = [e['name'] for e in ss.get_snaps() if 'checkbox-' in e['name']]
+            snaps = qabro.utils.get_snaps()
+            snap_list = [e.props.name for e in snaps if 'checkbox-' in e.props.name]
             for snap_name in snap_list:
-                sp = ["/home/$USER/snap/{}/current/.cache/plainbox/sessions",
-                      "/root/snap/{}/current/.cache/plainbox/sessions"]
+                sp = [os.path.join(usr_dir, "snap/{}/current/.cache/plainbox/sessions"),
+                      os.path.join(root_dir, "snap/{}/current/.cache/plainbox/sessions")]
                 for p in sp:
                     snap_path = os.path.expandvars(p.format(snap_name))
                     if os.path.exists(snap_path):
diff --git a/qabro/utils.py b/qabro/utils.py
index af790eb..89845b3 100644
--- a/qabro/utils.py
+++ b/qabro/utils.py
@@ -1,67 +1,12 @@
 #!/usr/bin/env python3
 import socket, json, pprint
 
-class snapdsocket:
-    def __init__(self, sock=None):
-        if sock is None:
-            self.sock = socket.socket(socket.AF_UNIX)
-        else:
-            self.sock = sock
-        self.sock.settimeout(0.3)
+import gi
+gi.require_version('Snapd', '1')
+from gi.repository import Snapd
 
-    def _connect(self):
-        self.sock.connect("/run/snapd.socket")
 
-    def _send(self, msg):
-        if type(msg) != "bytes":
-            msg = msg.encode()
-        self.sock.send(msg)
-
-    def _recv(self):
-        chunks = []
-        while True:
-            try:
-                chunk = self.sock.recv(2048)
-                chunks.append(chunk.decode())
-            except socket.timeout:
-                break
-        return ''.join(chunks)
-
-    def _close(self):
-        self.sock.close()
-
-    def _get_json_response(self, response):
-        begin = response.find('{')
-        end = response.rfind('}') + 1
-        return json.loads(response[begin:end])
-
-    def request(self, msg):
-        self.__init__()
-        self._connect()
-        self._send(msg)
-        raw_resp = self._recv()
-        self._close()
-        json_resp = self._get_json_response(raw_resp)
-        return json_resp
-
-    def get_interfaces(self):
-        req = "GET /v2/interfaces HTTP/1.1\r\nHost: localhost\r\n\r\n"
-        json_resp = self.request(req)
-        return json_resp['result']
-
-    def get_snaps(self):
-        req = "GET /v2/snaps HTTP/1.1\r\nHost: localhost\r\n\r\n"
-        json_resp = self.request(req)
-        return json_resp['result']
-
-    def get_snap_names(self):
-        req = "GET /v2/snaps HTTP/1.1\r\nHost: localhost\r\n\r\n"
-        json_resp = self.request(req)
-        snap_names = []
-        for s in json_resp["result"]:
-            snap_names.append(s["name"])
-        return snap_names
-
-if __name__ == "__main__":
-    sn = snapdsocket()
-    pprint.pprint(sn.get_snaps())
+def get_snaps():
+    client = Snapd.Client()
+    active_snaps_flag = Snapd.GetSnapsFlags(0)
+    return client.get_snaps_sync(active_snaps_flag, None)
diff --git a/setup.cfg b/setup.cfg
new file mode 100644
index 0000000..50c84a6
--- /dev/null
+++ b/setup.cfg
@@ -0,0 +1,6 @@
+[tool:pytest]
+testpaths = tests
+
+[coverage:run]
+branch = True
+source = qabro
diff --git a/setup.py b/setup.py
index d01f071..cd3c091 100644
--- a/setup.py
+++ b/setup.py
@@ -24,6 +24,10 @@ automatically attach relevent information to them (log files).
     entry_points={
         'console_scripts': ['qabro = qabro.__init__:main']
     },
+    install_requires=['launchpadlib', 'urwid'],
+    extras_require={
+        'test': ['pytest', 'pytest-mock', 'coverage', 'PyGObject'],
+    },
     author=about['__author__'],
     author_email=about['__author_email__'],
     url=about['__url__'],
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index de2580d..346125e 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -10,7 +10,8 @@ apps:
   qabro:
     command: qabro
     environment:
-      LD_LIBRARY_PATH: $LD_LIBRARY_PATH:$SNAP/lib/fwts:$SNAP/lib/x86_64-linux-gnu
+      GI_TYPELIB_PATH: $SNAP/usr/lib/$SNAPCRAFT_ARCH_TRIPLET/girepository-1.0
+      LD_LIBRARY_PATH: $LD_LIBRARY_PATH:$SNAP/lib/fwts:$SNAP/lib/$SNAPCRAFT_ARCH_TRIPLET
       LANG: C.UTF-8
       LC_ALL: C.UTF-8
 
@@ -42,6 +43,8 @@ parts:
       - pciutils
       - python3-urwid
       - ubuntu-report
+      - python3-gi
+      - gir1.2-snapd-1 # LP #1887903
 
   dpkg:
     plugin: nil
diff --git a/tests/test_bug_assistant.py b/tests/test_bug_assistant.py
new file mode 100644
index 0000000..752db7d
--- /dev/null
+++ b/tests/test_bug_assistant.py
@@ -0,0 +1,69 @@
+import pytest
+from qabro.bug_assistant import AttachmentAssistant
+import gi
+gi.require_version('Snapd', '1')
+from gi.repository import Snapd
+
+import os
+import pprint
+import sys
+pprint.pprint(sys.path)
+
+
[e-mail](scope="session")
+def get_snaps(session_mocker):
+    test_snap = session_mocker.MagicMock()
+    test_snap.props.name = "checkbox-snappy"
+    test_snap.props.version = "20.15"
+    test_snap.props.revision = "2035"
+    test_snap.props.tracking_channel = None
+    test_snap.props.devmode = True
+    test_snap.props.confinement.value_nick = "strict"
+
+    session_mocker.patch("qabro.utils.get_snaps", return_value=[test_snap, ])
+
+
[e-mail]("usr_dir",
+                         [".cache/plainbox/sessions",
+                          "snap/checkbox-snappy/current/.cache/plainbox/sessions"])
+def test_get_last_checkbox_session_path_ubuntu_classic(get_snaps, tmp_path, usr_dir):
+    for session in ["session1", "session2", "last_session"]:
+        s = tmp_path / usr_dir / session
+        s.mkdir(parents=True)
+    last_session_path = tmp_path / usr_dir / "last_session"
+    # Add 1 second to last session directory access/modify time to make sure
+    # it will be seen as the most recent session directory
+    last_session_time = os.stat(last_session_path).st_mtime + 1
+    os.utime(last_session_path, times=(last_session_time, last_session_time))
+    aa = AttachmentAssistant()
+    last_session = aa._get_last_checkbox_session_path(usr_dir=tmp_path, root_dir="/nothing/here")
+    assert last_session.endswith("last_session")
+
+
+def test_get_last_checkbox_session_path_ubuntu_core(get_snaps, tmp_path):
+    root_dir = "snap/checkbox-snappy/current/.cache/plainbox/sessions"
+    for session in ["session1", "session2", "last_session"]:
+        s = tmp_path / root_dir / session
+        s.mkdir(parents=True)
+    last_session_path = tmp_path / root_dir / "last_session"
+    # Add 1 second to last session directory access/modify time to make sure
+    # it will be seen as the most recent session directory
+    last_session_time = os.stat(last_session_path).st_mtime + 1
+    os.utime(last_session_path, times=(last_session_time, last_session_time))
+    aa = AttachmentAssistant()
+    last_session = aa._get_last_checkbox_session_path(usr_dir="/nothing/here", root_dir=tmp_path)
+    assert last_session.endswith("last_session")
+
+
+def test_get_last_checkbox_session_path_no_session(get_snaps, tmp_path):
+    aa = AttachmentAssistant()
+    last_session = aa._get_last_checkbox_session_path(usr_dir="/nothing/here", root_dir="/nothing/there")
+    assert last_session == ""
+
+
+def test_attach_snaplist(get_snaps):
+    aa = AttachmentAssistant()
+    aa.attach_snaplist()
+    test_output = (b"Name                            Version         Revision        Channel         Devmode         Confinement     \n"
+                   b"checkbox-snappy                 20.15           2035            -               yes             strict          \n")
+    assert aa.attachments["snap_list.log"] == test_output