Bug 93723

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 / TestsAssignee: 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 Flags
proposed fix
dpranke: review-
proposed fix dpranke: review+

Kristóf Kosztyó
Reported 2012-08-10 08:30:09 PDT
Now the nrwt doesn't warn if a test from the skipped list doesn't exist.
Attachments
proposed fix (1.36 KB, patch)
2012-08-15 06:33 PDT, Kristóf Kosztyó 		dpranke: review-
DetailsFormatted DiffDiff
proposed fix (3.04 KB, patch)
2012-08-28 08:14 PDT, Kristóf Kosztyó 		dpranke: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Dirk Pranke
Comment 1 2012-08-10 11:36:38 PDT
seems reasonable and a trivial change (test_expectations.py:889)
Kristóf Kosztyó
Comment 2 2012-08-15 06:33:46 PDT
Created attachment 158558 [details] proposed fix
Dirk Pranke
Comment 3 2012-08-15 11:18:48 PDT
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.
Kristóf Kosztyó
Comment 4 2012-08-28 08:14:13 PDT
Created attachment 160986 [details] proposed fix
Dirk Pranke
Comment 5 2012-08-28 12:35:10 PDT
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.
Kristóf Kosztyó
Comment 6 2012-08-29 06:09:50 PDT
Committed r126992: <http://trac.webkit.org/changeset/126992>
Note You need to log in before you can comment on or make changes to this bug.