Summary: | REGRESSION (NRWT): Results for new non-text-only tests are always put in the most-specific platform directory | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> | ||||
Component: | Tools / Tests | Assignee: | Dirk Pranke <dpranke> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, darin, dpranke, eric, hyatt, mjs, ojan | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 88680 | ||||||
Attachments: |
|
Description
Adam Roben (:aroben)
2012-02-08 09:55:09 PST
The guilty code in NRWT: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py?rev=106224#L163 port.baseline_path() ends up returning the most-specific platform directory. Hm. There seem to be plusses and minuses to ORWT's logic. I agree that tests should be as generic as possible. However, is there no way to put baselines in the most-specific directory with ORWT? If not, that would mean baseline locations would have to be managed by hand, right? The advantage to NRWT's behavior is that all platforms will put baselines in safe locations, and then a simple 'optimize' pass can eliminate duplicates. That said, I don't feel that strongly about it one way or another, and it's certainly easy to change. We could include the optimize logic in NRWT. Requiring people to run webkit-patch optimize-baselines after running NRWT seems cumbersome. (In reply to comment #3) > We could include the optimize logic in NRWT. Requiring people to run webkit-patch optimize-baselines after running NRWT seems cumbersome. I'm not sure how useful that would be, since NRWT will only generate one port's results, as opposed to something like garden-o-matic or webkit-patch rebaseline, which is likely to pull all of the baselines for a test. I would be somewhat concerned that optimizing the baselines might produce some amount of thrashing if we're doing one port at a time. Maybe not, though. I do agree that running a separate optimize step is a hassle, though. Maybe we should default to least-specific, and just have a separate flag for most-specific for the rare times when you need it? (Although I'm not sure when you would know it was needed and yet not just optimize or move things by hand). (In reply to comment #4) > Maybe we should default to least-specific, and just have a separate flag for most-specific for the rare times when you need it? (Although I'm not sure when you would know it was needed and yet not just optimize or move things by hand). ORWT has such a flag: --add-platform-exceptions (In reply to comment #5) > (In reply to comment #4) > > Maybe we should default to least-specific, and just have a separate flag for most-specific for the rare times when you need it? (Although I'm not sure when you would know it was needed and yet not just optimize or move things by hand). > > ORWT has such a flag: --add-platform-exceptions Ah. Alrighty then :). Created attachment 148204 [details]
Patch
Comment on attachment 148204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148204&action=review > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:284 > + optparse.make_option("--add-platform-exceptions", action="store_true", default=False, I would have named this flag --use-most-specific-directory or --use-version-directory. I don't understand what "exceptions" you're referring here. > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:977 > + # Test that we update the platform expectations in the version-specific dirs Nit: Please spell out directories. (In reply to comment #8) > (From update of attachment 148204 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=148204&action=review > > > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:284 > > + optparse.make_option("--add-platform-exceptions", action="store_true", default=False, > > I would have named this flag --use-most-specific-directory or --use-version-directory. I don't understand what "exceptions" you're referring here. > Yeah, it's not a great name. I'm using it for compatibility w/ ORWT. We should probably file a separate bug to rename --reset-results, --new-baseline, and --add-platform-exceptions all at once to something saner. > > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:977 > > + # Test that we update the platform expectations in the version-specific dirs > > Nit: Please spell out directories. Will do. Committed r120740: <http://trac.webkit.org/changeset/120740> |