Bug 60373

Summary:	check-webkit-style / TestExpectationChecker doesn't catch all parse errors in test_expectations.txt
Product:	WebKit	Reporter:	Adrienne Walker <enne>
Component:	Tools / Tests	Assignee:	Nobody <webkit-unassigned>
Status:	RESOLVED DUPLICATE
Severity:	Normal	CC:	abarth, dpranke, enne, levin
Priority:	P2
Version:	528+ (Nightly build)
Hardware:	Unspecified
OS:	Unspecified

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 ***