Per the design docs in http://trac.webkit.org/wiki/TestExpectations, [ Skip ] and [ WontFix ] are not supposed to allow any other modifiers, but I forgot to implement that before we converted the syntax.
Created attachment 164989 [details]
Committed r129171: <http://trac.webkit.org/changeset/129171>
Comment on attachment 164989 [details]
Don't we need to skip WontFix tests first? Otherwise, you'll get unexpected failures for WontFix tests.
WontFix implies Skip automatically (in the code today).
in the new syntax; in the old syntax, it didn't, i.e., when we translate the new syntax back into the old syntax, we add on a "SKIP".
making WONTFIX imply SKIP regardless of the syntax is what I'm working on right now; it unfortunately requires updating a bunch of unit tests in non trivial ways.
At least, I think the existing code does this; if not, I just broke the world ;).
Reopening to attach new patch.
Created attachment 165014 [details]
Note that the changed TestExpectations files were landed in http://trac.webkit.org/changeset/129171
Comment on attachment 165014 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=165014&action=review
> + return self.warnings and self.warnings != ['Test lacks BUG modifier.']
This is a bit too hacky I think. Could we at least move the string to a shared constant that both locations use?
(In reply to comment #11)
> (From update of attachment 165014 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=165014&action=review
> > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:442
> > + return self.warnings and self.warnings != ['Test lacks BUG modifier.']
> This is a bit too hacky I think. Could we at least move the string to a shared constant that both locations use?
Sure, good idea.
Created attachment 165023 [details]
add MISSING_BUG_WARNING constant for string reuse
Created attachment 165029 [details]
patch for landing - move warning
Committed r129184: <http://trac.webkit.org/changeset/129184>
This seems to have caused a webkitpy failure:
Commented out the failing unit test in http://trac.webkit.org/changeset/129188
(In reply to comment #17)
> Commented out the failing unit test in http://trac.webkit.org/changeset/129188
Ah, I forgot to change that to assertTrue(). Thanks!
Test fixed in http://trac.webkit.org/changeset/129245 .