Merge lp:~roadmr/checkbox/cmdline-subparser into lp:checkbox

[Daniel Manrique]

checkbox-support:parsers:submission: Add parsing of kernel_cmdline_attachment.

This includes the kernel commandline parser as well as the changes to SubmissionParser and the TestRun class to actually invoke the subparser when needed and gather the data, data files and tests.

checkbox-support:parsers:submission: Add parsing of kernel_cmdline_attachment.

This includes the kernel commandline parser as well as the changes to SubmissionParser and the TestRun class to actually invoke the subparser when needed and gather the data, data files and tests.

The parser is purposefully trivial since I was mostly trying to understand how to add and connect a new subparser in the SubmissionParser.

[Zygmunt Krynicki (zyga)]

Just minor nitpicks below. I'd put commits in different order though:
 - docstring for test run
 - new parser _with_ tests

[Daniel Manrique (roadmr)]

Applied the fixes as suggested and changed the commits too.

Exceptions:

- I didn't change the TestRun name as that is also used in Hexr and I want to be consistent, this is scary enough as it is :)
- I didn't change the camelcase for the setKernelCmdline method for the same reason. Let me know if you feel strongly about this and I can change it.
- I still haven't figured out the DeferredParser thing, once I got this parser working I'm investigating *why* it works so I'll be able to answer this later, hopefully...

[Zygmunt Krynicki (zyga)]

I'm fine with the first two as it's more of a long-term change. The
last thing is okay if you feel that this is the way things work. I was
more curious than anything and I assumed you know why you've used it
:-)

+1

On Fri, May 8, 2015 at 9:56 PM, Daniel Manrique
<email address hidden> wrote:
> Applied the fixes as suggested and changed the commits too.
>
> Exceptions:
>
> - I didn't change the TestRun name as that is also used in Hexr and I want to be consistent, this is scary enough as it is :)
> - I didn't change the camelcase for the setKernelCmdline method for the same reason. Let me know if you feel strongly about this and I can change it.
> - I still haven't figured out the DeferredParser thing, once I got this parser working I'm investigating *why* it works so I'll be able to answer this later, hopefully...
> --
> https://code.launchpad.net/~roadmr/checkbox/cmdline-subparser/+merge/258666
> You are subscribed to branch lp:checkbox.

[Zygmunt Krynicki (zyga)]

Thanks, let's merge it, +1

review: Approve

[Daniel Manrique (roadmr)]

Attempt to merge into lp:checkbox failed due to conflicts:

text conflict in checkbox-support/checkbox_support/parsers/submission.py
text conflict in checkbox-support/checkbox_support/parsers/tests/test_submission.py

[Daniel Manrique (roadmr)]

I knew that was going to happen :) Fixed the merge conflicts, ready again.

[lp:~roadmr/checkbox/cmdline-subparser]

3774. By Daniel Manrique on 2015-05-13

checkbox-support:parsers:submission: Add parsing of kernel_cmdline_attachment.

This includes the kernel commandline parser (trivial since we don't actually
parse the commandline, we just extract it verbatim from the raw attachment data)
as well as the changes to SubmissionParser and the TestRun class to actually invoke
the subparser when needed and gather the data.

Testing changes add the xml chunk data file for the test, the test
method itself, and augment the SubmissionRun data-holding class with a
setKernelCmdline method which we'd expect the submission parser to call
when it finds and parses a kernel_cmdline_attachment.

3775. By Daniel Manrique on 2015-05-13

checkbox-support:parsers:submission: Document the TestRun class and SubmissionParser.run method

=== added file 'checkbox-support/checkbox_support/parsers/kernel_cmdline.py'
--- checkbox-support/checkbox_support/parsers/kernel_cmdline.py	1970-01-01 00:00:00 +0000
+++ checkbox-support/checkbox_support/parsers/kernel_cmdline.py	2015-05-13 13:30:33 +0000
@@ -0,0 +1,37 @@
+# This file is part of Checkbox.
+#
+# Copyright 2015 Canonical Ltd.
+#
+# Checkbox is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3,
+# as published by the Free Software Foundation.
+#
+# Checkbox is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+
+class KernelCmdlineParser(object):
+
+    """Parser for kernel cmdline information."""
+
+    def __init__(self, stream):
+        self.stream = stream
+
+    def run(self, result):
+        """
+        The kernel cmdline is usually a single line of text so this parser is
+        quite simple. It will just call the result's setKernelCmdline method
+        with the first line
+        """
+        line = self.stream.readline().strip()
+        result.setKernelCmdline(line)
 
=== modified file 'checkbox-support/checkbox_support/parsers/submission.py'
--- checkbox-support/checkbox_support/parsers/submission.py	2015-05-12 18:08:20 +0000
+++ checkbox-support/checkbox_support/parsers/submission.py	2015-05-13 13:30:33 +0000
@@ -44,6 +44,7 @@
 from checkbox_support.parsers.meminfo import MeminfoParser
 from checkbox_support.parsers.udevadm import UdevadmParser
 from checkbox_support.parsers.modprobe import ModprobeParser
+from checkbox_support.parsers.kernel_cmdline import KernelCmdlineParser
 
 
 logger = logging.getLogger("checkbox_support.parsers.submission")
@@ -65,6 +66,47 @@
 # from apps/uploads/checkbox_parser.py licensed internally by Canonical under
 # the license of the Chcekbox project.
 class TestRun(object):
+    """
+    The TestRun class is responsible for acting upon information from a
+    submission. It decouples the storage and processing of that information
+    from the parsing process. A TestRun class is passed to the SubmissionParser
+    at run time::
+
+        # stream is the file or submission data
+        parser = SubmissionParser(stream)
+        parser.run(TestRun, <other arguments>)
+
+    The parser will create a TestRun instance and, as it finds elements
+    in the submission, will call methods in the TestRun instance passing them
+    the chunks it has parsed. The TestRun instance can do things like print
+    the data, save it into a list or dict for later use, dump it
+    directly to a database, or anything else.
+
+    The interface that TestRun-compliant classes must implement is not really
+    formalized anywhere; perhaps *this* class is the most authoritative
+    reference of which methods/events may be called.
+
+    This particular TestRun implementation uses "messages" as its storage
+    convention, for historical reasons: it's initialized with an empty
+    list and it will populate it with the data stored in dictionaries of
+    the form::
+
+        { type: "set-$something",
+          "foo": "data-1",
+          "baz": "data-2"}
+
+    The only required key is "type": the rest are dependent on which data
+    item is processed.
+
+    There are a few conventions in naming the "callback" methods:
+
+    - Methods that will be called only once to set a single item are
+      named set\* (example setArchitecture).
+    - Methods that can be called many times due to processing of several
+      similar items (packages, devices) are named add\*
+      (example addDeviceState). Look at the existing methods to see how they
+      append to an existing element of the messages list.
+    """
 
     project = "certification"
 
@@ -96,6 +138,12 @@
             "module": module,
             "options": options})
 
+    def setKernelCmdline(self, kernel_cmdline):
+        self.messages.append({
+            "type": "set-kernel-cmdline",
+            "kernel_cmdline": kernel_cmdline})
+        logger.debug("Setting Kernel Commandline: %s", kernel_cmdline)
+
     def setDistribution(self, **distribution):
         self.messages.append({
             "type": "set-distribution",
@@ -499,6 +547,9 @@
             ("cpuinfo", "machine", "cpuinfo_result",),
             self.setCpuinfo, count=1)
         register(
+            ("test_run", "kernel_cmdline",),
+            self.setKernelCmdline, count=1)
+        register(
             ("meminfo", "meminfo_result",),
             self.setMeminfo, count=1)
         register(
@@ -542,6 +593,7 @@
             r"udevadm": self.parseUdevadm,
             r"efi(?!rtvariable)": EfiParser,
             r"modprobe_attachment": self.parseModprobe,
+            r"kernel_cmdline": self.parseKernelCmdline,
             }
         for context, parser in context_parsers.items():
             if re.search(context, command):
@@ -656,6 +708,10 @@
         elif name == "kernel-release":
             self.dispatcher.publishEvent("kernel", value)
 
+    def parseKernelCmdline(self, cmdline):
+        self.dispatcher.publishEvent("kernel_cmdline", cmdline)
+        return DeferredParser(self.dispatcher, "kernel_cmdline_result")
+
     def parseCpuinfo(self, cpuinfo):
         self.dispatcher.publishEvent("cpuinfo", cpuinfo)
         return DeferredParser(self.dispatcher, "cpuinfo_result")
@@ -680,6 +736,10 @@
     def setKernelState(self, test_run, kernel):
         test_run.setKernelState(kernel)
 
+    def setKernelCmdline(self, test_run, kernel_cmdline):
+        parser = KernelCmdlineParser(kernel_cmdline)
+        parser.run(test_run)
+
     def setCpuinfo(self, cpuinfo, machine, cpuinfo_result):
         parser = CpuinfoParser(cpuinfo, machine)
         parser.run(cpuinfo_result)
@@ -1002,6 +1062,18 @@
                     "Unsupported tag <%s> in <system>" % child.tag)
 
     def run(self, test_run_factory, **kwargs):
+        """
+        Entry point to start parsing the stream with which the parser
+        was initialized.
+
+        :param test_run_factory: A class from which to instantiate a
+        "test_run" object whose add\*/set\* methods will be called as elements
+        are found in the stream
+
+        :returns: a SubmissionResult instance. This is not really used
+        and seems redundant, as the data will be processed and stored by
+        the TestRun instance (which is, however, also not returned anywhere).
+        """
         parser = etree.XMLParser()
 
         tree = etree.parse(self.file, parser=parser)
 
=== added file 'checkbox-support/checkbox_support/parsers/tests/fixtures/submission_info_kernel_cmdline.xml'
--- checkbox-support/checkbox_support/parsers/tests/fixtures/submission_info_kernel_cmdline.xml	1970-01-01 00:00:00 +0000
+++ checkbox-support/checkbox_support/parsers/tests/fixtures/submission_info_kernel_cmdline.xml	2015-05-13 13:30:33 +0000
@@ -0,0 +1,10 @@
+<?xml version="1.0" ?>
+<system version="1.0">
+    <context>
+        <info command="kernel_cmdline_attachment">BOOT_IMAGE=/boot/vmlinuz-3.13.0-48-generic root=UUID=645553f3-c6b8-49c4-adf5-68ac8134d9e5 ro quiet splash video.use_native_backlight=1 radeon.modeset=0 nouveau.modeset=0
+        </info>
+    </context>
+    <summary>
+        <architecture value="amd64"/>
+    </summary>
+</system>
 
=== modified file 'checkbox-support/checkbox_support/parsers/tests/test_submission.py'
--- checkbox-support/checkbox_support/parsers/tests/test_submission.py	2015-05-12 18:08:20 +0000
+++ checkbox-support/checkbox_support/parsers/tests/test_submission.py	2015-05-13 13:30:33 +0000
@@ -54,6 +54,9 @@
         self.result.setdefault('module_options', {})
         self.result['module_options'][module] = options
 
+    def setKernelCmdline(self, kernel_cmdline):
+        self.result['kernel_cmdline'] = kernel_cmdline
+
     def addAttachment(self, **attachment):
         self.result.setdefault("attachments", [])
         self.result["attachments"].append(attachment)
@@ -187,6 +190,13 @@
         self.assertIn("single_cmd=1",
                       result['module_options']['snd-hda-intel'])
 
+    def test_kernel_cmdline(self):
+        """a kernel commandline can be in a kernel_cmdline info element."""
+        result = self.getResult("submission_info_kernel_cmdline.xml")
+        self.assertTrue("kernel_cmdline" in result)
+        self.assertIn("ro quiet splash video.use_native_backlight=1",
+                      result["kernel_cmdline"])
+
     def test_device_dmidecode(self):
         """Device states can be in a dmidecode info element."""
         result = self.getResult("submission_info_dmidecode.xml")