Bug 60373 - check-webkit-style / TestExpectationChecker doesn't catch all parse errors in test_expectations.txt
Summary: check-webkit-style / TestExpectationChecker doesn't catch all parse errors in...
Status: RESOLVED DUPLICATE of bug 60466
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-06 09:17 PDT by Adrienne Walker
Modified: 2011-05-09 13:23 PDT (History)
4 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adrienne Walker 2011-05-06 09:17:03 PDT 
It's unfortunate that it's so easy to check in a test_expectations.txt file that doesn't work either due to a typo or due to two rules that conflict with each other in specificity.  It'd be nice if folks were warned that they were about to do such a thing.
Comment 1 Adam Barth 2011-05-06 09:37:36 PDT 
Another approach is to simply test_expectations to the point where it cannot have parse errors.
Comment 2 Adrienne Walker 2011-05-06 09:39:20 PDT 
(In reply to comment #1)
> Another approach is to simply test_expectations to the point where it cannot have parse errors.

This bug was prompted by http://trac.webkit.org/changeset/85950, which fixed what was called a parse error, but it was really a cuplicate expectation where one was more specific than the other.

On further inspection, it looks like check-webkit-style will complain about "duplicate or ambiguous expectation" but only if both offending lines are in the patch.  Maybe it needs to consider the whole file?
Comment 3 Adam Barth 2011-05-06 09:41:53 PDT 
I don't believe it is possible to have parse errors in the Skipped files, which are another approach to the same problem.  Yes, I know Skipped files do not have all the features of test_expectations.
Comment 4 David Levin 2011-05-06 10:07:07 PDT 
(In reply to comment #2)
> (In reply to comment #1)
> > Another approach is to simply test_expectations to the point where it cannot have parse errors.
> 
> This bug was prompted by http://trac.webkit.org/changeset/85950, which fixed what was called a parse error, but it was really a cuplicate expectation where one was more specific than the other.
> 
> On further inspection, it looks like check-webkit-style will complain about "duplicate or ambiguous expectation" but only if both offending lines are in the patch.  Maybe it needs to consider the whole file?

In general, check-webkit-style only complains about the line that you changed. However, I'm sure one could change this for a particular checker.

Patches welcome :).
Comment 5 Dirk Pranke 2011-05-06 10:39:34 PDT 
I think this should probably be merged into bug 51548, and I agree that the checker needs to consider the whole file, not just the lines that are modified.
Comment 6 David Levin 2011-05-09 13:23:50 PDT 
This particular issue (of filtering errors) came up for another checker as well, so I'm fixing both in one bug.

*** This bug has been marked as a duplicate of bug 60466 ***