Summary: | only include failures in full_results.json | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||
Component: | New Bugs | Assignee: | Ojan Vafai <ojan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, dpranke, eric, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Ojan Vafai
2011-04-22 16:38:16 PDT
Created attachment 90801 [details]
Patch
Comment on attachment 90801 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90801&action=review > Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:128 > + test_dict = {} It would be nice to just make these structs at some point. > Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:130 > + if False: > + test_dict['has_stderr'] = True Debugging code? Comment on attachment 90801 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90801&action=review > Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:74 > + """Summarize failing results as a dict. Unless I'm misunderstanding this, it looks like this change removes unexpected passes. That's bad - it'll break the bot output reporting unexpected passes and the log output for the same. It seems to me that if you want to exclude the results from the JSON file you should do it where you build the JSON file, not by changing this function. (In reply to comment #4) > (From update of attachment 90801 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=90801&action=review > > > Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:74 > > + """Summarize failing results as a dict. > > Unless I'm misunderstanding this, it looks like this change removes unexpected passes. That's bad - it'll break the bot output reporting unexpected passes and the log output for the same. Good catch. Fixed. > It seems to me that if you want to exclude the results from the JSON file you should do it where you build the JSON file, not by changing this function. I don't see why. The only places the summarized results are used is to write out the json and print out unexpected failures. I don't really see that changing in the future, do you? Committed r85095: <http://trac.webkit.org/changeset/85095> (In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 90801 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=90801&action=review > > > > > Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:74 > > > + """Summarize failing results as a dict. > > > > Unless I'm misunderstanding this, it looks like this change removes unexpected passes. That's bad - it'll break the bot output reporting unexpected passes and the log output for the same. > > Good catch. Fixed. > > > It seems to me that if you want to exclude the results from the JSON file you should do it where you build the JSON file, not by changing this function. > > I don't see why. The only places the summarized results are used is to write out the json and print out unexpected failures. I don't really see that changing in the future, do you? At some point I'd like to change the Manager.run() routine to return something like the summarized results so that we can write better integration tests, but it's a pretty low-priority thing. Nit, since the summarized results kept the unexpected passes, the original docstring was more accurate. I think you forgot to change it back. http://trac.webkit.org/changeset/85095 might have broken Windows 7 Release (Tests) |