Summary: | Use a hierarchical data structure to store results JSON | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||||
Component: | New Bugs | Assignee: | Ojan Vafai <ojan> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | dpranke, eric, mihaip, tony | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Ojan Vafai
2011-04-28 14:07:34 PDT
Created attachment 91553 [details]
Patch
Comment on attachment 91553 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91553&action=review Are you going to bump the version number? > Tools/Scripts/webkitpy/common/net/resultsjsonparser_unittest.py:58 > "skipped": 450, Do we need to update the version number for these results? > Tools/Scripts/webkitpy/layout_tests/layout_package/printing.py:448 > + def _print_unexpected_results_subtree(self, tree, passes, flaky, regressions, prefix=""): Seems we should share code with LayoutTestResults/JSONTestResult, no? Comment on attachment 91553 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91553&action=review > LayoutTests/ChangeLog:5 > + use a heirarchical data structure to store results json Should probably fix the typo everywhere :) > LayoutTests/fast/harness/resources/results-test.js:116 > + var subtree = {} Seems cleaner to say var subtree = results.tests['foo'] = {}; (below too) > LayoutTests/fast/harness/results.html:311 > + globalState().testsWithStderr.push(test); Seems like the initial part of tableRow is about initializing the global state from the tests, and doesn't actually have anything to do with table rows. You may want to move that to a separate function (can be in a different patch). > Tools/Scripts/webkitpy/tool/commands/rebaselineserver.py:400 > + def _gather_current_baselines(self, tree, test_config, new_tests_subtree, prefix=""): Use single quotes, to be consistent with the rest of the code. (In reply to comment #2) > (From update of attachment 91553 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=91553&action=review > > Are you going to bump the version number? Sure. In practice, this is just here so we have the option of writing code that understands both version as we transition. I'm changing all the places where we read in the JSON, so it doesn't really matter. > > Tools/Scripts/webkitpy/common/net/resultsjsonparser_unittest.py:58 > > "skipped": 450, > > Do we need to update the version number for these results? May as well. > > Tools/Scripts/webkitpy/layout_tests/layout_package/printing.py:448 > > + def _print_unexpected_results_subtree(self, tree, passes, flaky, regressions, prefix=""): > > Seems we should share code with LayoutTestResults/JSONTestResult, no? Yeah. I couldn't think of a great place to put it and it's only 5 lines of code. But it is in 4 places! I'm moving it to being a free function in resultsjsonparser. (In reply to comment #3) > (From update of attachment 91553 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=91553&action=review > > > LayoutTests/ChangeLog:5 > > + use a heirarchical data structure to store results json > > Should probably fix the typo everywhere :) Sigh. English is hard. :( > > LayoutTests/fast/harness/resources/results-test.js:116 > > + var subtree = {} > > Seems cleaner to say var subtree = results.tests['foo'] = {}; > > (below too) Will do. > > LayoutTests/fast/harness/results.html:311 > > + globalState().testsWithStderr.push(test); > > Seems like the initial part of tableRow is about initializing the global state from the tests, and doesn't actually have anything to do with table rows. You may want to move that to a separate function (can be in a different patch). May as well do it here. Created attachment 91571 [details]
Patch
Committed r85254: <http://trac.webkit.org/changeset/85254> Committed r85255: <http://trac.webkit.org/changeset/85255> |