[Piglit] [PATCH] Optionally capture dmesg changes for each test and report them in a summary

Marek Olšák maraeo at gmail.com
Fri Sep 20 07:29:25 PDT 2013


I think your implementation of dmesg logging isn't correct. Note that
dmesg is a ring buffer; once it's full, new lines added at the end of
dmesg will cause the lines at the beginning to disappear. That's why I
had to find and save the timestamp string, and find its position again
in a new dmesg log.

Marek

On Wed, Sep 18, 2013 at 11:17 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> On Wednesday 18 September 2013 14:16:11 you wrote:
>> From: Marek Olšák <maraeo at gmail.com>
>>
>> The Radeon driver writes GPU page faults to dmesg and we need to know which
>> test caused them.
>>
>> If there is any change in dmesg during a test run, the test result is
>> changed as follows:
>> * pass -> dmesg-warn
>> * warn -> dmesg-warn
>> * fail -> dmesg-fail
>> Dmesg is captured before and after the test and the difference between the
>> two is stored in the test summary.
>>
>> The piglit-run.py parameter which enables this behavior is --dmesg. It's
>> also recommended to use -c0.
>>
>> v2: - Use a simplified class to read dmesg
>>
>> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
>> ---
>>  framework/core.py          |  5 +++--
>>  framework/dmesg.py         | 48
>> ++++++++++++++++++++++++++++++++++++++++++++++
> framework/exectest.py      |
>> 20 ++++++++++++++-----
>>  framework/shader_test.py   |  4 ++--
>>  framework/summary.py       | 26 ++++++++++++++++++-------
>>  piglit-run.py              |  6 +++++-
>>  templates/index.css        |  6 +++++-
>>  templates/test_result.mako |  7 ++++++-
>>  8 files changed, 103 insertions(+), 19 deletions(-)
>>  create mode 100644 framework/dmesg.py
>>
>> diff --git a/framework/core.py b/framework/core.py
>> index 25e84c1..0d7bbde 100644
>> --- a/framework/core.py
>> +++ b/framework/core.py
>> @@ -362,13 +362,14 @@ class TestrunResult:
>>
>>  class Environment:
>>      def __init__(self, concurrent=True, execute=True, include_filter=[],
>> -                 exclude_filter=[], valgrind=False):
>> +                 exclude_filter=[], valgrind=False, dmesg=False):
>>          self.concurrent = concurrent
>>          self.execute = execute
>>          self.filter = []
>>          self.exclude_filter = []
>>          self.exclude_tests = set()
>>          self.valgrind = valgrind
>> +        self.dmesg = dmesg
>>
>>          """
>>          The filter lists that are read in should be a list of string
>> objects, @@ -446,7 +447,7 @@ class Test:
>>              try:
>>                  status("running")
>>                  time_start = time.time()
>> -                result = self.run(env.valgrind)
>> +                result = self.run(env.valgrind, env.dmesg)
>>                  time_end = time.time()
>>                  if 'time' not in result:
>>                      result['time'] = time_end - time_start
>> diff --git a/framework/dmesg.py b/framework/dmesg.py
>> new file mode 100644
>> index 0000000..659f3eb
>> --- /dev/null
>> +++ b/framework/dmesg.py
>> @@ -0,0 +1,48 @@
>> +# Copyright (c) 2013 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.
>> +
>> +
>> +import subprocess
>> +
>> +
>> +class Dmesg(object):
>> +    def __init__(self):
>> +        self.index = 0
>> +        self.__update_dmesg()
>> +
>> +    def __call__(self):
>> +        """
>> +        Calling dmesg updates the object by running the dmesg command
>> again. It +        then returns either a populated list with the changes,
>> or an empty list +        """
>> +        self.__update_dmesg()
>> +        return self.contents
>> +
>> +    def __update_dmesg(self):
>> +        """
>> +        Call dmesg and store the output.
>> +        Track the element index number, and only return entries new than
>> that +        index.
>> +        """
>> +        proc = subprocess.Popen(['dmesg'], stdout=subprocess.PIPE)
>> +        dmesg = proc.communicate()[0].rstrip('\n').split('\n')
>> +        self.contents = dmesg[self.index:]
>> +        self.index = len(dmesg)
>> diff --git a/framework/exectest.py b/framework/exectest.py
>> index 6ee550c..c604f86 100644
>> --- a/framework/exectest.py
>> +++ b/framework/exectest.py
>> @@ -27,6 +27,7 @@ import shlex
>>  import types
>>
>>  from core import Test, testBinDir, TestResult
>> +from dmesg import Dmesg
>>
>>
>>  # Platform global variables
>> @@ -36,7 +37,7 @@ else:
>>      PIGLIT_PLATFORM = ''
>>
>>
>> -# ExecTest: A shared base class for tests that simply run an executable.
>> +# ExecTest: A shared base class for tests that simply runs an executable.
>>  class ExecTest(Test):
>>      def __init__(self, command):
>>          Test.__init__(self)
>> @@ -49,11 +50,11 @@ class ExecTest(Test):
>>
>>          self.skip_test = self.check_for_skip_scenario(command)
>>
>> -    def interpretResult(self, out, returncode, results):
>> +    def interpretResult(self, out, returncode, results, dmesg):
>>          raise NotImplementedError
>>          return out
>>
>> -    def run(self, valgrind):
>> +    def run(self, valgrind, dmesg):
>>          """
>>          Run a test.  The return value will be a dictionary with keys
>>          including 'result', 'info', 'returncode' and 'command'.
>> @@ -81,6 +82,7 @@ class ExecTest(Test):
>>                      err = ""
>>                      returncode = None
>>                  else:
>> +                    dmesg = Dmesg()
>>                      (out, err, returncode) = \
>>                          self.get_command_result(command, fullenv)
>>
>> @@ -117,7 +119,7 @@ class ExecTest(Test):
>>                  results['result'] = 'skip'
>>              else:
>>                  results['result'] = 'fail'
>> -                out = self.interpretResult(out, returncode, results)
>> +                out = self.interpretResult(out, returncode, results, dmesg)
>>
>>              crash_codes = [
>>                  # Unix: terminated by a signal
>> @@ -161,6 +163,7 @@ class ExecTest(Test):
>>                                                               err, out)
>>              results['returncode'] = returncode
>>              results['command'] = ' '.join(self.command)
>> +            results['dmesg'] = dmesg.contents
>>
>>              self.handleErr(results, err)
>>
>> @@ -217,11 +220,18 @@ class PlainExecTest(ExecTest):
>>          # Prepend testBinDir to the path.
>>          self.command[0] = testBinDir + self.command[0]
>>
>> -    def interpretResult(self, out, returncode, results):
>> +    def interpretResult(self, out, returncode, results, dmesg):
>>          outlines = out.split('\n')
>>          outpiglit = map(lambda s: s[7:],
>>                          filter(lambda s: s.startswith('PIGLIT:'),
>> outlines))
>>
>> +        # Using the __call__ method here returns either an empty list, or a
>> +        # list with the relevant dmesg entries
>> +        if dmesg():
>> +            outpiglit = map(lambda s: s.replace("'pass'", "'dmesg-warn'"),
>> outpiglit) +            outpiglit = map(lambda s: s.replace("'warn'",
>> "'dmesg-warn'"), outpiglit) +            outpiglit = map(lambda s:
>> s.replace("'fail'", "'dmesg-fail'"), outpiglit) +
>>          if len(outpiglit) > 0:
>>              try:
>>                  for piglit in outpiglit:
>> diff --git a/framework/shader_test.py b/framework/shader_test.py
>> index b80af24..c91c22e 100755
>> --- a/framework/shader_test.py
>> +++ b/framework/shader_test.py
>> @@ -243,7 +243,7 @@ class ShaderTest(PlainExecTest):
>>          self.__command = [runner] + self.__shader_runner_args
>>          return self.__command
>>
>> -    def run(self, valgrind=False):
>> +    def run(self, valgrind=False, dmesg=False):
>>          """ Parse the test file's [require] block to determine which
>>          executable is needed to run the test. Then run the executable on
>> the test file."""
>> @@ -259,7 +259,7 @@ class ShaderTest(PlainExecTest):
>>                  # parsing the test file discovered an error.
>>                  return self.__result
>>
>> -            return PlainExecTest.run(self, valgrind)
>> +            return PlainExecTest.run(self, valgrind, dmesg)
>>
>>
>>  def _usage_error():
>> diff --git a/framework/summary.py b/framework/summary.py
>> index 1cdbab7..5ce327c 100644
>> --- a/framework/summary.py
>> +++ b/framework/summary.py
>> @@ -325,10 +325,14 @@ class Summary:
>>                      return 2
>>                  elif status == 'warn':
>>                      return 3
>> -                elif status == 'fail':
>> +                elif status == 'dmesg-warn':
>>                      return 4
>> -                elif status == 'crash':
>> +                elif status == 'fail':
>>                      return 5
>> +                elif status == 'dmesg-fail':
>> +                    return 6
>> +                elif status == 'crash':
>> +                    return 7
>>
>>              openGroup('fake')
>>              openGroup('all')
>> @@ -421,12 +425,16 @@ class Summary:
>>                  return 1
>>              elif status == 'warn':
>>                  return 2
>> -            elif status == 'fail':
>> +            elif status == 'dmesg-warn':
>>                  return 3
>> -            elif status == 'skip':
>> +            elif status == 'fail':
>>                  return 4
>> -            elif status == 'crash':
>> +            elif status == 'dmesg-fail':
>>                  return 5
>> +            elif status == 'skip':
>> +                return 6
>> +            elif status == 'crash':
>> +                return 7
>>              elif status == 'special':
>>                  return 0
>>
>> @@ -450,7 +458,7 @@ class Summary:
>>                      # If the result contains a value other than 1 (pass) or
>> 4 # (skip) it is a problem. Skips are not problems becasuse # they have
>> Their own page.
>> -                    if [i for e in [2, 3, 5] for i in status if e is i]:
>> +                    if [i for e in [2, 3, 4, 5, 7] for i in status if e is
>> i]: self.tests['problems'].append(test)
>>
>>                  if 'skipped' in lists:
>> @@ -480,7 +488,8 @@ class Summary:
>>          Private: Find the total number of pass, fail, crash, skip, and warn
>> in the *last* set of results stored in self.results.
>>          """
>> -        self.totals = {'pass': 0, 'fail': 0, 'crash': 0, 'skip': 0, 'warn':
>> 0} +        self.totals = {'pass': 0, 'fail': 0, 'crash': 0, 'skip': 0,
>> 'warn': 0, +                       'dmesg-warn': 0, 'dmesg-fail': 0}
>>
>>          for test in self.results[-1].tests.values():
>>              self.totals[test['result']] += 1
>> @@ -547,6 +556,7 @@ class Summary:
>>                          info=value.get('info', 'None'),
>>                          traceback=value.get('traceback', 'None'),
>>                          command=value.get('command', 'None'),
>> +                        dmesg=value.get('dmesg', 'None'),
>>                          css=path.relpath(resultCss, tPath),
>>                          index=path.relpath(index, tPath)))
>>                      file.close()
>> @@ -625,6 +635,8 @@ class Summary:
>>          print "      crash: %d" % self.totals['crash']
>>          print "       skip: %d" % self.totals['skip']
>>          print "       warn: %d" % self.totals['warn']
>> +        print "       dmesg-warn: %d" % self.totals['dmesg-warn']
>> +        print "       dmesg-fail: %d" % self.totals['dmesg-fail']
>>          if self.tests['changes']:
>>              print "    changes: %d" % len(self.tests['changes'])
>>              print "      fixes: %d" % len(self.tests['fixes'])
>> diff --git a/piglit-run.py b/piglit-run.py
>> index 33318d4..7e99791 100755
>> --- a/piglit-run.py
>> +++ b/piglit-run.py
>> @@ -72,6 +72,9 @@ def main():
>>      parser.add_argument("--valgrind",
>>                          action="store_true",
>>                          help="Run tests in valgrind's memcheck")
>> +    parser.add_argument("--dmesg",
>> +                        action="store_true",
>> +                        help="Capture a difference in dmesg before and
>> after each test") parser.add_argument("testProfile",
>>                          metavar="<Path to test profile>",
>>                          help="Path to testfile to run")
>> @@ -108,7 +111,8 @@ def main():
>>                             exclude_filter=args.exclude_tests,
>>                             include_filter=args.include_tests,
>>                             execute=args.execute,
>> -                           valgrind=args.valgrind)
>> +                           valgrind=args.valgrind,
>> +                           dmesg=args.dmesg)
>>
>>      # Change working directory to the root of the piglit directory
>>      piglit_dir = path.dirname(path.realpath(sys.argv[0]))
>> diff --git a/templates/index.css b/templates/index.css
>> index 875333f..577370c 100644
>> --- a/templates/index.css
>> +++ b/templates/index.css
>> @@ -36,7 +36,7 @@ td:first-child > div {
>>       background-color: #c8c838
>>  }
>>
>> -td.skip, td.warn, td.fail, td.pass, td.trap, td.abort, td.crash {
>> +td.skip, td.warn, td.fail, td.pass, td.trap, td.abort, td.crash,
>> td.dmesg-warn, td.dmesg-fail { text-align: right;
>>  }
>>
>> @@ -59,9 +59,13 @@ tr:nth-child(even) td.skip  { background-color: #a0a0a0;
>> }
>>
>>  tr:nth-child(odd)  td.warn  { background-color: #ff9020; }
>>  tr:nth-child(even) td.warn  { background-color: #ef8010; }
>> +tr:nth-child(odd)  td.dmesg-warn  { background-color: #ff9020; }
>> +tr:nth-child(even) td.dmesg-warn  { background-color: #ef8010; }
>>
>>  tr:nth-child(odd)  td.fail  { background-color: #ff2020; }
>>  tr:nth-child(even) td.fail  { background-color: #e00505; }
>> +tr:nth-child(odd)  td.dmesg-fail  { background-color: #ff2020; }
>> +tr:nth-child(even) td.dmesg-fail  { background-color: #e00505; }
>>
>>  tr:nth-child(odd)  td.trap  { background-color: #111111; }
>>  tr:nth-child(even) td.trap  { background-color: #000000; }
>> diff --git a/templates/test_result.mako b/templates/test_result.mako
>> index 410dbb4..b23fb8e 100644
>> --- a/templates/test_result.mako
>> +++ b/templates/test_result.mako
>> @@ -46,7 +46,12 @@
>>            <pre>${traceback}</pre>
>>          </td>
>>        </tr>
>> -
>> +      <tr>
>> +        <td>dmesg</td>
>> +        <td>
>> +          <pre>${dmesg}</pre>
>> +        </td>
>> +      </tr>
>>      </table>
>>      <p><a href="${index}">Back to summary</a></p>
>>    </body>
>
> So I got bored last night and implemented a class as I'd suggested. I like it,
> but you're free to ignore it.


More information about the Piglit mailing list