On Tue, Feb 4, 2014 at 1:12 PM, Dylan Baker <[email protected]> wrote: > On Tuesday, February 04, 2014 12:43:42 PM Ilia Mirkin wrote: >> On Tue, Feb 4, 2014 at 12:35 PM, Dylan Baker <[email protected]> > 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 <[email protected]> >> > --- >> > >> > 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 <[email protected]> >> >> > + >> > + 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 >> > [email protected] >> > 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. _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
