[Piglit] [PATCH 1/4] framework/dmesg: Adds a new module for dmesg
Ilia Mirkin
imirkin at alum.mit.edu
Fri Jan 31 17:39:31 PST 2014
On Fri, Jan 31, 2014 at 8:02 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
> seperate 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. The third
> advantage is performance, this module doesn't search for timestamps and
> compare from there, instead it creates snapshots of dmesg as lists, and
> does a linear comparison between them.
>
> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> ---
> framework/dmesg.py | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 157 insertions(+)
> create mode 100644 framework/dmesg.py
>
> diff --git a/framework/dmesg.py b/framework/dmesg.py
> new file mode 100644
> index 0000000..81410d6
> --- /dev/null
> +++ b/framework/dmesg.py
> @@ -0,0 +1,157 @@
> +# 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 """
> +
> +import os
> +import subprocess
> +import itertools
> +
> +__all__ = ['get_dmesg']
> +
> +
> +class PosixDmesg(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.
> +
> + This class is not thread-safe, but with the caveat that the test reporting
> + dmesg problems might not be the test that generated them. dmesg reporting
> + can only be considered authoritative when running non-concurrently.
> +
> + """
> + DMESG_COMMAND = ['dmesg']
> +
> + def __init__(self):
> + """ Create a dmesg instance """
> + self._dmesg = []
> + self._last_message = ""
> + self._new_messages = []
I suppose a reason for not double-underscoring these will become apparent later?
> +
> + # Populate self.dmesg initially, otherwise the first test will always
> + # be full of dmesg crud.
> + self.update_dmesg()
> +
> + 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
> +
> + """
> + assert(isinstance(result, dict))
Doesn't _really_ have to be a dict, does it? And any plain ol' dict
won't work either since you expect certain keys to be there. I'd drop
this.
> +
> + def replace(res):
> + """ helper to replace statuses with the new dmesg status
> +
> + Replaces pass with dmesg-warn, and warn and fail with dmesg-fail.
> +
> + """
> + if res == "pass":
> + return 'dmesg-warn'
> + elif res in ['warn', 'fail']:
> + return 'dmesg-fail'
> + return res
A slightly faster way to do this might be:
return {
"pass": "dmesg-warn",
"warn": "dmesg-fail",
"fail": "dmesg-fail",
}.get(res, res)
Although it's also a bit fancier, so perhaps not worth it.
> +
> + # 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
> + try:
> + for key, value in result['subtest'].iteritems():
> + result['subtest'][key] = replace(value)
> + except KeyError:
> + pass
Is this really cleaner than
for key, value in result.get('subtest', {}).iteritems()
or
if 'subtest' in result:
for ...
Or is the try/catch protecting against something subtle I'm not thinking of?
> +
> + # 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).splitlines()
> +
> + # Calculate new entires in dmesg by returning any strings that are in
> + # the new dmesg list but not in the old one. This circumvents the
> + # ringbuffer issue, since if there are not entries in common all of the
> + # new messages will be returned.
Can you explain how this works? Let's say that dmesg can store 3 lines
(yeah, i know it's bytes, but wtvr), self._dmesg would be a, b, c.
Then a new line is emitted, and it becomes b, c, d. itertools.izip
will have new/old pairs of (a, b), (b, c), (c, d) -- it'll all appear
to be new. Am I misreading the code/misunderstanding how izip works?
Basically I don't see a "try to line things up" step. Also note that
because it actually _is_ bytes and not lines, the very first line
might be truncated (its first characters, not its last ones).
> + self._new_messages = []
> + for new, old in itertools.izip_longest(dmesg, self._dmesg, fillvalue=None):
> + if new == old:
> + continue
> + self._new_messages.append(new)
> +
> +
> + # Now that we have checked the entries of self._dmesg, we can update
> + # that member with the contents of the latest read of dmesg
> + self._dmesg = dmesg
> +
> +
> +class DummyDmesg(object):
> + """ An dummy class for dmesg on non unix-like systems
> +
> + This implements a dummy version of the PosixDmesg, 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 os.name == "posix" and not_dummy:
> + return PosixDmesg()
> + return DummyDmesg()
> --
> 1.8.5.3
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list