| Summary: | [NRWT] The nrwt should check the contents of the skipped files with --lint-test-files | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kristóf Kosztyó <kkristof> | ||||||
| Component: | Tools / Tests | Assignee: | Kristóf Kosztyó <kkristof> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, bank, dpranke, kadam, ojan, ossy, szledan, webkit.review.bot | ||||||
| Priority: | P2 | Keywords: | NRWT | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 64491 | ||||||||
| Attachments: |
|
||||||||
|
Description
Kristóf Kosztyó
2012-08-10 08:30:09 PDT
seems reasonable and a trivial change (test_expectations.py:889) Created attachment 158558 [details]
proposed fix
Comment on attachment 158558 [details]
proposed fix
expectation_for_skipped_test() won't set .warnings, so it seems unlikely that this branch would ever execute. Also, checking for one warning and then printing another seems like a bad idea.
You should just test if the file (or directory) exists here and add a warning directly (or add that code into expectation_for_skipped_test().
Also, this change needs a test.
Created attachment 160986 [details]
proposed fix
Comment on attachment 160986 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=160986&action=review > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:96 > + _log.warning('The following test %s from the Skipped list doesn\'t exists!' % test_name) Nit: "exist", not "exists", and I wouldn't use an exclamation point. Committed r126992: <http://trac.webkit.org/changeset/126992> |