Merge lp:~sylvain-pineau/checkbox/device_check into lp:checkbox

[Sylvain Pineau]

This is a result of the QA roundtable we had last week in Taipei.

Context:

When a device is not detected on a system, all tests that should have been generated are simply not here and could lead to a valid certificate. It's very hard to review a submission and look for missing tests. But it's far easier to review a submission including failed tests.

This MR proposes a new test using the udev parser but in a new way. With the new --list option it enumerates devices per categories (e.g VIDEO, WIRELESS). The tester can then compare this list with the official manifest of the machine and then can fail the test and help the certificate reviewer.

So far only VIDEO, NETWORK, WIRELESS and DISK categories are considered but it's not limited and could be extended to all categories known to the udev parser.

I'll later propose to add this test to all certification test plans as a mandatory job. It will work on both client and server since the tee commands also redirect to stdout (zenity will only be run on desktop).

[Sylvain Pineau (sylvain-pineau)]

Sample output from my own system:

===========================[ Running Selected Jobs ]============================
--------------[ Running job 1 / 1. Estimated time left: 0:00:05 ]---------------
--------------------------------[ Device Check ]--------------------------------
ID: 2013.com.canonical.certification::miscellanea/device_check
Category: 2013.com.canonical.plainbox::miscellanea
Purpose:

Device check

Steps:

1. Commence the test
2. Check the system manifest and review the devices known to udev

Pick an action
    => press ENTER to continue
  c => add a comment
  s => skip this job
  q => save the session and quit
[csq]:
... 8< -------------------------------------------------------------------------
Gtk-Message: GtkDialog mapped without a transient parent. This is discouraged.
VIDEO (1):
 - Intel Corporation 3rd Gen Core processor Graphics Controller

NETWORK (2):
 - Ericsson Business Mobile Networks BV H5321 gw
 - Intel Corporation 82579LM Gigabit Network Connection

WIRELESS (1):
 - Intel Corporation Centrino Ultimate-N 6300 (3x3 AGN)

DISK (1):
 - None HGST HTS725050A7E630

------------------------------------------------------------------------- >8 ---
Verification:

Are all devices well reported by udev?

Please decide what to do next:
  outcome: job needs verification
  comments:
Pick an action
  c => add a comment
  p => set outcome to pass
  f => set outcome to fail
  s => set outcome to skip
  r => re-run this job
    => set suggested outcome [job passed]
[cpfsr]: p
Outcome: job passed
==================================[ Results ]===================================
 ☑ : Device Check

[Jeff Lane  (bladernr)]

Just so I understand this, all you're basically doing is dumping UDEV by category, and then manually comparing that to a piece of paper that lists what should be on the SUT?

I'm curious because this KINDA answers something I've been trying to think of a solution for in Server as well. I just want to make sure I understand your description of how this works.

[Sylvain Pineau (sylvain-pineau)]

Yes this is exactly that. Sorry if my wording what not clear enough. As said, some jobs are not even created if a device is not reported by udev (to be clear again they are not skipped or failed just not created at all).

What the test is adding is a manual check, a job meant to be run before anything else to leave an evidence and ease the review.

[Jeff Lane  (bladernr)]

Thanks! Actually, your wording was fine, I just saw this first thing this morning before coffee and wanted to be sure, because as I said, it's an idea that ties in with something I've wanted to solve myself with server as well... so I'll have to look at co-opting this later on :)

review: Needs Fixing

[lp:~sylvain-pineau/checkbox/device_check]

4323. By Launchpad Translations on behalf of checkbox-dev on 2016-05-02

Launchpad automatic translations update.

4324. By Sylvain Pineau on 2016-05-02

providers:resource:udev: Add the --list option to enumerate device per category

4325. By Sylvain Pineau on 2016-05-02

providers:checkbox: New job meant to check devices reported by udev

This new job relies on the udev_resource command to list devices by categories.
It will help the tester to review the SUT manifest and fail/comment on the test
if devices are missing.

[Sylvain Pineau (sylvain-pineau)]

I've added a choice list - hardcoded though - which should cover more than we need since we're using a very limited set of devices in template jobs.

review: Needs Resubmitting

[Jeff Lane  (bladernr)]

Great!

review: Approve

=== modified file 'providers/plainbox-provider-checkbox/jobs/miscellanea.txt.in'
--- providers/plainbox-provider-checkbox/jobs/miscellanea.txt.in	2016-04-20 22:12:24 +0000
+++ providers/plainbox-provider-checkbox/jobs/miscellanea.txt.in	2016-05-02 19:03:03 +0000
@@ -286,3 +286,17 @@
 command: get_make_and_model
 _description: Retrieve the computer's make and model for easier access than digging through the dmidecode output.
 _summary: Gather info on the SUT's make and model
+
+plugin: user-interact-verify
+category_id: 2013.com.canonical.plainbox::miscellanea
+estimated_duration: 5.0
+id: miscellanea/device_check
+command: udev_resource -l VIDEO NETWORK WIRELESS DISK | tee >([[ $DISPLAY ]] && zenity --text-info --title="Device report")
+_summary: Device Check
+_purpose:
+    Device check
+_steps:
+    1. Commence the test
+    2. Compare items on System Manifest to the devices known to udev
+_verification:
+    Do the devices reported by udev match the devices on the Manifest?

=== modified file 'providers/plainbox-provider-resource-generic/bin/udev_resource'
--- providers/plainbox-provider-resource-generic/bin/udev_resource	2016-02-02 12:54:35 +0000
+++ providers/plainbox-provider-resource-generic/bin/udev_resource	2016-05-02 19:03:03 +0000
@@ -20,12 +20,17 @@
 import argparse
 import shlex
 
+from collections import OrderedDict
 from subprocess import check_output, CalledProcessError
 
 from checkbox_support.parsers.udevadm import UdevadmParser
 
-
-class UdevResult:
+categories = ("ACCELEROMETER", "AUDIO", "BLUETOOTH", "CAPTURE", "CARDREADER",
+              "DISK", "KEYBOARD", "MOUSE", "NETWORK", "OTHER", "TOUCHPAD",
+              "TOUCHSCREEN", "USB", "VIDEO", "WIRELESS")
+
+
+class UdevResultDump:
 
     attributes = ("path", "name", "bus", "category", "driver", "product_id",
                   "vendor_id", "subproduct_id", "subvendor_id", "product",
@@ -40,6 +45,29 @@
         print()
 
 
+class UdevResultLister:
+
+    def __init__(self, categories):
+        self.categories = categories
+        self._data = OrderedDict()
+        for c in categories:
+            self._data[c] = []
+
+    def display(self):
+        for c, devices in self._data.items():
+            print("{} ({}):".format(c, len(devices)))
+            for d in devices:
+                print(" - {}".format(d))
+            print()
+
+    def addDevice(self, device):
+        c = getattr(device, "category", None)
+        if c in self.categories:
+            p = getattr(device, "product", "Unknow product")
+            v = getattr(device, "vendor", "Unknow vendor")
+            self._data[c].append("{} {}".format(v, p))
+
+
 def main():
     parser = argparse.ArgumentParser()
     parser.add_argument("-c", "--command", action='store', type=str,
@@ -50,6 +78,12 @@
                         default="lsblk -i -n -P -o KNAME,TYPE,MOUNTPOINT",
                         help="""Command to execute to get lsblk information.
                               Only change it if you know what you're doing.""")
+    parser.add_argument('-l','--list', nargs='+', choices=categories,
+                        metavar=("CATEGORY"),
+                        help="""List devices found under the requested
+                        categories.
+                        Acceptable categories to list are: 
+                        {}""".format(', '.join(categories)))
     args = parser.parse_args()
     try:
         output = check_output(shlex.split(args.command))
@@ -61,8 +95,13 @@
     output = output.decode("UTF-8", errors='ignore')
     lsblk = lsblk.decode("UTF-8", errors='ignore')
     udev = UdevadmParser(output, lsblk=lsblk)
-    result = UdevResult()
-    udev.run(result)
+    if args.list:
+        result = UdevResultLister(args.list)
+        udev.run(result)
+        result.display()
+    else:
+        result = UdevResultDump()
+        udev.run(result)
 
 
 if __name__ == "__main__":