| Summary: | check-webkit-style of the chromium test_expectations.txt file takes too long | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||||
| Component: | New Bugs | Assignee: | Ojan Vafai <ojan> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, dglazkov, dpranke, levin, rniwa, tony, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 76749 | ||||||||
| Attachments: |
|
||||||||
|
Description
Ojan Vafai
2012-01-20 15:03:05 PST
Created attachment 123386 [details]
Patch
I think the right approach would be to just reuse the model, instead of caching. The model no longer depends on the port/platform against which you check, so it should just be able to take multiple passes. That's how I envisioned it working, anyway. That's why the errors are collected for each line -- so that you could review them all once you ran them against all ports. Created attachment 123428 [details]
Patch
Comment on attachment 123428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123428&action=review I'll let dglazkov do the actual review. Just some style nits. > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:795 > if len(errors) or len(warnings): Nit: I think you can remove the len() calls. > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:799 > if len(errors): Ditto. > Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:220 > + def test_error_on_different_platform(self): Do we have a test that checks different configurations (e.g., DEBUG vs RELEASE)? Comment on attachment 123428 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123428&action=review >> Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:220 >> + def test_error_on_different_platform(self): > > Do we have a test that checks different configurations (e.g., DEBUG vs RELEASE)? I think it might be good to capture just a few more expectations here, just in case the machinery in layout_tests/models is refactored away or something. Committed r105770: <http://trac.webkit.org/changeset/105770> Committed r105789: <http://trac.webkit.org/changeset/105789> |