Merge lp:~pwlars/lava-test/demo-parse into lp:lava-test/0.0

[Paul Larson]

I want to make this more widely known, but this wasn't precisely what I wanted to merge. This version includes a cmd_parse class to add a command that I don't intend to actually add. It is just for demonstration purposes, but I could possibly see committing it for now, then pulling it out later. The actual parsing would not be a manual step that the user needs to enter a command for, but would rather happen as part of the results submission process. In the interest of moving forward though, I'd like to get this committed soon, so that I don't end up with too many branches queued up.

[James Westby (james-w)]

29 + testdata = json.loads(file(testdatafile,'r').read())
30 + test = abrek.testdef.testloader(testdata['testname'])
31 + try:
32 + test.parse(self.args[0]

Why read all the data and then not pass it to parse?

51 +class StreamTestParser(abrek.testdef.AbrekTestParser):
52 + def parse(self):
53 + super(StreamTestParser, self).parse()
54 + self.appendtoall({'units':'MB/s'})

This class isn't used, is it necessary?

104 + the parse() method should be called while already in the results
105 + directory and assumes that a file for test output will exist called
106 + testoutput.log.

Why not take a file descriptor with the test results? It's more flexible,
and chdir() isn't a great idea in library code.

122 + appenall=

typo in append.

182 + t['result'] = fixupdict[t['result']]

Do you want to assume that fixupdict will contain a key for every
result that will be found?

155 + def append(self,testid,entry):

Please add spaces after the comments in argument lists.

127 + self.results = {'testlist':[]}

Why a dict containing a list, and not just a list?

Seeing as you plan to remove cmd_parse I don't mind it missing tests,
but perhaps we should consider tests for the test_definitions?

Thanks,

James

review: Needs Information

[Paul Larson (pwlars)]

> Why read all the data and then not pass it to parse?
I need to read that bit to figure out which test this is exactly, so
that I can load the right parser. The parser then acts on the actual
test output

> This class isn't used, is it necessary?
 No, it was a leftover from a previous revision. I really thought I
had removed it. I will certainly do that now

> Why not take a file descriptor with the test results? It's more flexible,
> and chdir() isn't a great idea in library code.
I know, I don't like the chdir either, but I wanted to handle cases that are outside the norm more easily. For instance, if for some reason there is more than one output file that needs to be parsed. It would be useful to let the test definition extend the AbrekTestParser class and have it handle whatever files it needs to in the parse() method.

> typo in append.
Fixed

> Do you want to assume that fixupdict will contain a key for every
> result that will be found?
Typically, I expect it probably will. But there's always some chance
that you need to convert from a result format that is almost right,
except for one kind of result. Or a better example might be where the
results you get are good for things like pass, fail, etc, but you have
some odd result that you need to categorize differently.

> Please add spaces after the comments in argument lists.
Done

> Why a dict containing a list, and not just a list?
This is going to get combined with other test data for the submission, of which the testlist is just one thing.

> Seeing as you plan to remove cmd_parse I don't mind it missing tests,
> but perhaps we should consider tests for the test_definitions?
I'm not sure it would serve a
terribly useful purpose. The test_definitions are using the classes
defined elsewhere that I do test - and those are the things I want to
keep from breaking. If the test_definitions break, it would more likely
be due to something changing in the test that they download. That's why
I would like to target a specific version of the test. But if I were to
write a unit test for it, it would have to be based on assumptions about
the version of the test we know about today. So the test_definition
could still be broken by a new version, but the unit test wouldn't help
catch that.

review: Needs Resubmitting

[lp:~pwlars/lava-test/demo-parse]

16. By Paul Larson on 2010-07-26

Some minor cleanups

[James Westby (james-w)]

review: Approve

=== modified file 'abrek/builtins.py'
--- abrek/builtins.py	2010-07-16 17:09:35 +0000
+++ abrek/builtins.py	2010-07-26 14:22:47 +0000
@@ -1,3 +1,4 @@
+import json
 import os
 import sys
 from optparse import make_option
@@ -5,6 +6,7 @@
 import abrek.command
 import abrek.testdef
 
+
 class cmd_version(abrek.command.AbrekCmd):
     """
     Show the version of abrek
@@ -69,6 +71,23 @@
             print "Test execution error: %s" % strerror
             sys.exit(1)
 
+class cmd_parse(abrek.command.AbrekCmd):
+    def run(self):
+        if len(self.args) != 1:
+            print "please specify the name of the result dir"
+            sys.exit(1)
+        config = abrek.config.AbrekConfig()
+        resultsdir = os.path.join(config.resultsdir,self.args[0])
+        testdatafile = os.path.join(resultsdir,"testdata.json")
+        testdata = json.loads(file(testdatafile,'r').read())
+        test = abrek.testdef.testloader(testdata['testname'])
+        try:
+            test.parse(self.args[0])
+        except Exception as strerror:
+            print "Test parse error: %s" % strerror
+            sys.exit(1)
+        print test.parser.results
+
 class cmd_uninstall(abrek.command.AbrekCmd):
     """
     Uninstall a test

=== modified file 'abrek/test_definitions/stream.py'
--- abrek/test_definitions/stream.py	2010-06-28 20:23:02 +0000
+++ abrek/test_definitions/stream.py	2010-07-26 14:22:47 +0000
@@ -4,8 +4,11 @@
 MD5="b6cd43b848e0d8b0824703369392f3c5"
 INSTALLSTEPS = ['cc stream.c -O2 -fopenmp -o stream']
 RUNSTEPS = ['./stream']
+PATTERN = "^(?P<testid>\w+):\W+(?P<result>\d+\.\d+)"
 
 streaminst = abrek.testdef.AbrekTestInstaller(INSTALLSTEPS, url=URL, md5=MD5)
 streamrun = abrek.testdef.AbrekTestRunner(RUNSTEPS)
+streamparser = abrek.testdef.AbrekTestParser(PATTERN,
+                             appendall={'units':'MB/s'})
 testobj = abrek.testdef.AbrekTest(testname="stream", installer=streaminst,
-                                  runner=streamrun)
+                                  runner=streamrun, parser=streamparser)

=== modified file 'abrek/testdef.py'
--- abrek/testdef.py	2010-07-14 21:36:48 +0000
+++ abrek/testdef.py	2010-07-26 14:22:47 +0000
@@ -1,6 +1,7 @@
 import hashlib
 import json
 import os
+import re
 import shutil
 import sys
 import time
@@ -91,14 +92,17 @@
         self.runner.run(self.resultsdir)
         self._savetestdata()
 
-    def parse(self,results):
+    def parse(self, resultname):
         if not self.parser:
             raise RuntimeError("no test parser defined for '%s'" %
                                 self.testname)
-        self.parser.parse(results)
+        self.resultsdir = os.path.join(self.config.resultsdir, resultname)
+        os.chdir(self.resultsdir)
+        self.parser.parse()
 
 class AbrekTestInstaller(object):
     """Base class for defining an installer object.
+
     This class can be used as-is for simple installers, or extended for more
     advanced funcionality.
 
@@ -181,6 +185,92 @@
         self._runsteps(resultsdir)
         self.endtime = datetime.utcnow()
 
+class AbrekTestParser(object):
+    """Base class for defining a test parser
+
+    This class can be used as-is for simple results parsers, but will
+    likely need to be extended slightly for many.  If used as it is,
+    the parse() method should be called while already in the results
+    directory and assumes that a file for test output will exist called
+    testoutput.log.
+
+    pattern - regexp pattern to identify important elements of test output
+        For example: If your testoutput had lines that look like:
+            "test01:  PASS", then you could use a pattern like this:
+            "^(?P<testid>\w+):\W+(?P<result>\w+)"
+            This would result in identifying "test01" as testid and "PASS"
+            as result.  Once parse() has been called, self.results.testlist[]
+            contains a list of dicts of all the key,value pairs found for
+            each test result
+    fixupdict - dict of strings to convert test results to standard strings
+        For example: if you want to standardize on having pass/fail results
+            in lower case, but your test outputs them in upper case, you could
+            use a fixupdict of something like: {'PASS':'pass','FAIL':'fail'}
+    appendall - Append a dict to the testlist entry for each result.
+        For example: if you would like to add units="MB/s" to each result:
+            appendall={'units':'MB/s'}
+    """
+    def __init__(self, pattern=None, fixupdict=None, appendall={}):
+        self.pattern = pattern
+        self.fixupdict = fixupdict
+        self.results = {'testlist':[]}
+        self.appendall = appendall
+
+    def _find_testid(self, id):
+        for x in self.results['testlist']:
+            if x['testid'] == id:
+                return self.results['testlist'].index(x)
+
+    def parse(self):
+        """Parse test output to gather results
+
+        Use the pattern specified when the class was instantiated to look
+        through the results line-by-line and find lines that match it.
+        Results are then stored in self.results.  If a fixupdict was supplied
+        it is used to convert test result strings to a standard format.
+        """
+        filename = "testoutput.log"
+        pat = re.compile(self.pattern)
+        with open(filename, 'r') as fd:
+            for line in fd.readlines():
+                match = pat.search(line)
+                if match:
+                    self.results['testlist'].append(match.groupdict())
+        if self.fixupdict:
+            self.fixresults(self.fixupdict)
+        if self.appendall:
+            self.appendtoall(self.appendall)
+
+    def append(self, testid, entry):
+        """Appends a dict to the testlist entry for a specified testid
+
+        This lets you add a dict to the entry for a specific testid
+        entry should be a dict, updates it in place
+        """
+        index = self._find_testid(testid)
+        self.results['testlist'][index].update(entry)
+
+    def appendtoall(self, entry):
+        """Append entry to each item in the testlist.
+
+        entry - dict of key,value pairs to add to each item in the testlist
+        """
+        for t in self.results['testlist']:
+            t.update(entry)
+
+    def fixresults(self, fixupdict):
+        """Convert results to a known, standard format
+
+        pass it a dict of keys/values to replace
+        For instance:
+            {"TPASS":"pass", "TFAIL":"fail"}
+        This is really only used for qualitative tests
+        """
+        for t in self.results['testlist']:
+            if t.has_key("result"):
+                t['result'] = fixupdict[t['result']]
+
+
 def testloader(testname):
     """
     Load the test definition, which can be either an individual

=== modified file 'tests/__init__.py'
--- tests/__init__.py	2010-07-09 21:49:54 +0000
+++ tests/__init__.py	2010-07-26 14:22:47 +0000
@@ -4,7 +4,8 @@
     module_names = ['tests.test_builtins',
                     'tests.test_abrekcmd',
                     'tests.test_abrektestinstaller',
-                    'tests.test_abrektestrunner']
+                    'tests.test_abrektestrunner',
+                    'tests.test_abrektestparser']
     loader = unittest.TestLoader()
     suite = loader.loadTestsFromNames(module_names)
     return suite

=== added file 'tests/test_abrektestparser.py'
--- tests/test_abrektestparser.py	1970-01-01 00:00:00 +0000
+++ tests/test_abrektestparser.py	2010-07-26 14:22:47 +0000
@@ -0,0 +1,49 @@
+import os
+import shutil
+import tempfile
+import unittest
+
+from abrek.testdef import AbrekTestParser
+
+class testAbrekTestParser(unittest.TestCase):
+    def setUp(self):
+        self.origdir = os.path.abspath(os.curdir)
+        self.tmpdir = tempfile.mkdtemp()
+        self.filename = os.path.abspath(__file__)
+        os.chdir(self.tmpdir)
+
+    def tearDown(self):
+        os.chdir(self.origdir)
+        shutil.rmtree(self.tmpdir)
+
+    def makeparser(self, *args, **kwargs):
+        return AbrekTestParser(*args, **kwargs)
+
+    def writeoutputlog(self, str):
+        with open("testoutput.log", "a") as fd:
+            fd.write(str)
+
+    def test_parse(self):
+        pattern = "^(?P<testid>\w+):\W+(?P<result>\w+)"
+        self.writeoutputlog("test001: pass")
+        parser = self.makeparser(pattern)
+        parser.parse()
+        self.assertTrue(parser.results["testlist"][0]["testid"] == "test001" and
+                        parser.results["testlist"][0]["result"] == "pass")
+
+    def test_fixupdict(self):
+        pattern = "^(?P<testid>\w+):\W+(?P<result>\w+)"
+        fixup = {"pass":"PASS"}
+        self.writeoutputlog("test001: pass")
+        parser = self.makeparser(pattern, fixupdict=fixup)
+        parser.parse()
+        self.assertEquals("PASS", parser.results["testlist"][0]["result"])
+
+    def test_appendall(self):
+        pattern = "^(?P<testid>\w+):\W+(?P<result>\w+)"
+        append = {"units":"foo/s"}
+        self.writeoutputlog("test001: pass")
+        parser = self.makeparser(pattern, appendall=append)
+        parser.parse()
+        self.assertEqual("foo/s", parser.results["testlist"][0]["units"])
+