[Piglit] [Patch v3 1/4] framework/dmesg: Adds a new module for dmesg

[Ilia Mirkin]

On Mon, Feb 3, 2014 at 6:40 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> This modules implements two classes and a helper function. The two
> classes are both dmesg wrapper classes, one, PosixDmesg is used on posix
> systems when requested to actually do checks on dmesg. The second class,
> DummyDmesg, is used to reduce code complexity by providing dummy
> versions of the methods in PosixDmesg. This means that we don't need
> separate code paths for Posix systems wanting to check dmesg, Posix
> systems not wanting to check dmesg, and non-posix systems which lack
> dmesg.
>
> Beyond reducing complexity this module also gives better isolation,
> dmesg is only used in Test.execute(), no where else. Additional classes
> don't need to worry about dmesg that way, it's just available.
>
> v2: - Remove unnecessary assert from PosixDmesg.update_result()
>     - simplify replace helper in PoixDmesg.update_result()
>     - replace try/except with if check
>     - Change PosixDmesg.update_dmesg() to work even if dmesg 'wraps'
> v3: - Try/Except assignment of PosixDmesg._last_message in update_dmesg()
>     - Set the dmesg levels the same as the previous implementation
>     - Change PosixDmesg into LinuxDmesg and enforce that dmesg has
>       timestamps
>
> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> ---
>  framework/dmesg.py | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 167 insertions(+)
>  create mode 100644 framework/dmesg.py
>
> diff --git a/framework/dmesg.py b/framework/dmesg.py
> new file mode 100644
> index 0000000..d270aaa
> --- /dev/null
> +++ b/framework/dmesg.py
> @@ -0,0 +1,167 @@
> +# Copyright (c) 2013-2014 Intel Corporation
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +""" Module implementing classes for reading posix dmesg
> +
> +Currently this module only has the default DummyDmesg, and a LinuxDmesg.
> +The method used on Linux requires that timetamps are enabled, and no other
> +posix system has timestamps.
> +
> +On OSX and *BSD one would likely want to implement a system that reads the
> +sysloger, since timestamps can be added by the sysloger, and are not inserted
> +by dmesg.
> +
> +"""
> +
> +import re
> +import sys
> +import subprocess
> +
> +__all__ = ['LinuxDmesg', 'DummyDmesg', 'get_dmesg']
> +
> +
> +class LinuxDmesg(object):
> +    """ Read dmesg on posix systems
> +
> +    This reads the dmesg ring buffer, stores the contents of the buffer, and
> +    then compares it whenever called, returning the difference between the
> +    calls. It requires timestamps to be enabled.
> +
> +    """
> +    DMESG_COMMAND = ['dmesg', '--level', 'emerg,alert,crit,err,warn,notice']
> +
> +    def __init__(self):
> +        """ Create a dmesg instance """
> +        self._last_message = None
> +        self._new_messages = []
> +
> +        # Populate self.dmesg initially, otherwise the first test will always
> +        # be full of dmesg crud.
> +        self.update_dmesg()
> +
> +        # Do an initial check to ensure that dmesg has timestamps, we need
> +        # timestamps to work correctly. A proper linux dmesg timestamp looks
> +        # like this: [    0.00000]
> +        if not re.match('\[\s*\d+\.\d+\]', self._last_message):

So if I do dmesg -c; ./piglit-run bla this will fail, right? (I'm
surprised that the string works without the r prefix for raw string,
but I double-checked and it does...)

> +            raise EnvironmentError("Your kernel does not seem to support "
> +                                   "timestamps, which are required by the "
> +                                   "--dmesg option")
> +
> +    def update_result(self, result):
> +        """ Takes a TestResult object and updates it with dmesg statuses
> +
> +        If dmesg is enabled, and if dmesg has been updated, then replace pass
> +        with dmesg-warn and warn and fail with dmesg-fail. If dmesg has not
> +        been updated, it will return the original result passed in.
> +
> +        Arguments:
> +        result -- A TestResult instance
> +
> +        """
> +
> +        def replace(res):
> +            """ helper to replace statuses with the new dmesg status
> +
> +            Replaces pass with dmesg-warn, and warn and fail with dmesg-fail,
> +            otherwise returns the input
> +
> +            """
> +            return {"pass": "dmesg-warn",
> +                    "warn": "dmesg-fail",
> +                    "fail": "dmesg-fail"}.get(res, res)
> +
> +        # Get a new snapshot of dmesg
> +        self.update_dmesg()
> +
> +        # if update_dmesg() found new entries replace the results of the test
> +        # and subtests
> +        if self._new_messages:
> +            result['result'] = replace(result['result'])
> +
> +            # Replace any subtests
> +            if 'subtest' in result:
> +                for key, value in result['subtest'].iteritems():
> +                    result['subtest'][key] = replace(value)
> +
> +            # Add the dmesg values to the result
> +            result['dmesg'] = self._new_messages
> +
> +        return result
> +
> +    def update_dmesg(self):
> +        """ Call dmesg using subprocess.check_output
> +
> +        Get the contents of dmesg, then calculate new messages, finally set
> +        self.dmesg equal to the just read contents of dmesg.
> +
> +        """
> +        dmesg = subprocess.check_output(self.DMESG_COMMAND).strip().splitlines()
> +
> +        # Find all new entries, do this by slicing the list of dmesg to only
> +        # returns elements after the last element stored. If there are not
> +        # matches a value error is raised, that means all of dmesg is new
> +        try:
> +            self._new_messages = dmesg[dmesg.index(self._last_message) + 1:]

IMO it'd still be an improvement to search from the end -- the vast
majority of the time, when a piglit test does not emit a new message,
the _last_message will match up to the last element in dmesg. Instead
of checking n items (n = # of lines in dmesg), you'd be only checking
1...

> +        except ValueError:
> +            self._new_messages = dmesg
> +
> +        # Attempt to store the contents of the last message in the dmesg
> +        # snapshot for lookup later. If something happens inbetween (say a user
> +        # calls dmesg -C), then set it to None

... or if dmesg is clean at the start -- it doesn't have to be someone
clearing it in the middle of the run.

> +        try:
> +            self._last_message = dmesg[-1]
> +        except IndexError:
> +            self._last_message = None

Completely optional suggestion:

self._last_message = dmesg[-1] if dmesg else None

(In general I prefer to avoid exceptions since you can end up catching
things you didn't mean to. But that's not everyone's style, so feel
free to do it whichever way you prefer.)

> +
> +
> +class DummyDmesg(object):
> +    """ An dummy class for dmesg on non unix-like systems
> +
> +    This implements a dummy version of the LinuxDmesg, and can be used anytime
> +    it is not desirable to actually read dmesg, such as non-posix systems, or
> +    when the contents of dmesg don't matter.
> +
> +    """
> +    DMESG_COMMAND = []
> +
> +    def __init__(self):
> +        pass
> +
> +    def update_dmesg(self):
> +        """ Dummy version of update_dmesg """
> +        pass
> +
> +    def update_result(self, result):
> +        """ Dummy version of update_result """
> +        return result
> +
> +
> +def get_dmesg(not_dummy=True):
> +    """ Return a Dmesg type instance
> +
> +    Normally this does a system check, and returns the type that proper for
> +    your system. However, if Dummy is True then it will always return a
> +    DummyDmesg instance.
> +
> +    """
> +    if sys.platform.startswith('linux') and not_dummy:
> +        return LinuxDmesg()
> +    return DummyDmesg()
> --
> 1.8.5.3
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit