Merge lp:~pwlars/lava-test/demo-parse into lp:lava-test/0.0
Proposed by
Paul Larson
Status: | Merged |
---|---|
Merged at revision: | 19 |
Proposed branch: | lp:~pwlars/lava-test/demo-parse |
Merge into: | lp:lava-test/0.0 |
Diff against target: |
250 lines (+166/-4) 5 files modified
abrek/builtins.py (+19/-0) abrek/test_definitions/stream.py (+4/-1) abrek/testdef.py (+92/-2) tests/__init__.py (+2/-1) tests/test_abrektestparser.py (+49/-0) |
To merge this branch: | bzr merge lp:~pwlars/lava-test/demo-parse |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Westby (community) | Approve | ||
Paul Larson (community) | Needs Resubmitting | ||
Review via email: mp+30688@code.launchpad.net |
Description of the change
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.
To post a comment you must log in.
29 + testdata = json.loads( file(testdatafi le,'r') .read() ) testloader( testdata[ 'testname' ]) self.args[ 0]
30 + test = abrek.testdef.
31 + try:
32 + test.parse(
Why read all the data and then not pass it to parse?
51 +class StreamTestParse r(abrek. testdef. AbrekTestParser ): tParser, self).parse() l({'units' :'MB/s' })
52 + def parse(self):
53 + super(StreamTes
54 + self.appendtoal
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