[Piglit] [Patch v5 1/4] framework/dmesg: Adds a new module for dmesg
Dylan Baker
baker.dylan.c at gmail.com
Tue Feb 4 10:37:56 PST 2014
On Tuesday, February 04, 2014 01:28:56 PM Ilia Mirkin wrote:
> On Tue, Feb 4, 2014 at 1:12 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> > On Tuesday, February 04, 2014 12:43:42 PM Ilia Mirkin wrote:
> >> On Tue, Feb 4, 2014 at 12:35 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
> >> >
> >> > v4: - Add check in LinuxDmesg.__init__ to ensure there is something in
> >> >
> >> > dmesg before doing the timestamp check. This should make problems
> >> > clearer
> >> >
> >> > - Refactor some code in LinuxDmesg.update_result()
> >> >
> >> > v5: - Replace exception with warning.
> >> >
> >> > - add break to loop in dmesg.update_dmesg
> >> >
> >> > Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> >> > ---
> >> >
> >> > framework/dmesg.py | 175
> >> > +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed,
> >> > 175 insertions(+)
> >> > create mode 100644 framework/dmesg.py
> >> >
> >> > diff --git a/framework/dmesg.py b/framework/dmesg.py
> >> > new file mode 100644
> >> > index 0000000..feb05ad
> >> > --- /dev/null
> >> > +++ b/framework/dmesg.py
> >> > @@ -0,0 +1,175 @@
> >> > +# 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
> >> > +import warnings
> >> > +
> >> > +__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()
> >> > +
> >> > + if not self._last_message:
> >> > + # We need to ensure that there is something in dmesg to
> >> > check
> >> > + # timestamps.
> >> > + warnings.warn("Cannot check dmesg for timestamp support.
> >> > If
> >> > you " + "do not have timestamps enabled in
> >> > your
> >> > kernel you " + "get incomplete dmesg
> >> > captures",
> >> > RuntimeWarning) + elif not re.match(r'\[\s*\d+\.\d+\]',
> >> > self._last_message): + # 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]
> >> > + raise DmesgError(
> >> > + "Your kernel does not seem to support timestamps,
> >> > which "
> >> > + "are required by the --dmesg option")
> >>
> >> Consider also making this a warning.
> >>
> >> Either way, Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
> >>
> >> > +
> >> > + 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 + l = 0
> >> > + for index, item in enumerate(reversed(dmesg)):
> >> > + if item == self._last_message:
> >> > + l = len(dmesg) - index # don't include the matched
> >> > element + break
> >> > + self._new_messages = dmesg[l:]
> >> > +
> >> > + # Attempt to store the last element of dmesg, unless there was
> >> > no
> >> > dmesg + self._last_message = dmesg[-1] if dmesg else None
> >> > +
> >> > +
> >> > +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
> >> > +
> >> > +
> >> > +class DmesgError(EnvironmentError):
> >> > + pass
> >> > +
> >> > +
> >> > +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
> >
> > Is that review for the series?
>
> It's a review for this patch. I believe I also reviewed 3/4 and 4/4
> earlier on (and those reviews should still stand). 2/4 uses some fancy
> testing framework I'm not familiar with, so I haven't looked at it.
> But the impact is also low, so probably fine to commit unreviewed?
> Dunno what the policy is.
Well, the policy is pretty much wait a few weeks, hope for a review, after a
few weeks if no one naks it push it. The nose tests I'm not super concerned
about review on. Nose is pretty cool, I'm much more impressed with it than the
built-in unittest stuff.
Thanks for reviewing all of this.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140204/9497e30b/attachment.pgp>
More information about the Piglit
mailing list