WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
97225
make Skip, WontFix be the only expectations on a line
https://bugs.webkit.org/show_bug.cgi?id=97225
Summary
make Skip, WontFix be the only expectations on a line
Dirk Pranke
Reported
2012-09-20 09:25:49 PDT
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.
Attachments
Patch
(241.47 KB, patch)
2012-09-20 14:51 PDT
,
Dirk Pranke
no flags
Details
Formatted Diff
Diff
Patch
(21.40 KB, patch)
2012-09-20 16:56 PDT
,
Dirk Pranke
no flags
Details
Formatted Diff
Diff
add MISSING_BUG_WARNING constant for string reuse
(22.36 KB, patch)
2012-09-20 17:53 PDT
,
Dirk Pranke
no flags
Details
Formatted Diff
Diff
patch for landing - move warning
(21.78 KB, patch)
2012-09-20 18:26 PDT
,
Dirk Pranke
ojan
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Dirk Pranke
Comment 1
2012-09-20 14:51:12 PDT
Created
attachment 164989
[details]
Patch
Dirk Pranke
Comment 2
2012-09-20 14:59:32 PDT
Committed
r129171
: <
http://trac.webkit.org/changeset/129171
>
Ojan Vafai
Comment 3
2012-09-20 15:22:25 PDT
Comment on
attachment 164989
[details]
Patch Don't we need to skip WontFix tests first? Otherwise, you'll get unexpected failures for WontFix tests.
Dirk Pranke
Comment 4
2012-09-20 15:31:20 PDT
WontFix implies Skip automatically (in the code today).
Dirk Pranke
Comment 5
2012-09-20 15:32:02 PDT
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".
Dirk Pranke
Comment 6
2012-09-20 15:33:26 PDT
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.
Dirk Pranke
Comment 7
2012-09-20 15:35:30 PDT
At least, I think the existing code does this; if not, I just broke the world ;).
Dirk Pranke
Comment 8
2012-09-20 16:56:22 PDT
Reopening to attach new patch.
Dirk Pranke
Comment 9
2012-09-20 16:56:24 PDT
Created
attachment 165014
[details]
Patch
Dirk Pranke
Comment 10
2012-09-20 16:57:08 PDT
Note that the changed TestExpectations files were landed in
http://trac.webkit.org/changeset/129171
Ojan Vafai
Comment 11
2012-09-20 17:04:06 PDT
Comment on
attachment 165014
[details]
Patch 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?
Dirk Pranke
Comment 12
2012-09-20 17:48:13 PDT
(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.
Dirk Pranke
Comment 13
2012-09-20 17:53:19 PDT
Created
attachment 165023
[details]
add MISSING_BUG_WARNING constant for string reuse
Dirk Pranke
Comment 14
2012-09-20 18:26:00 PDT
Created
attachment 165029
[details]
patch for landing - move warning
Dirk Pranke
Comment 15
2012-09-20 18:33:10 PDT
Committed
r129184
: <
http://trac.webkit.org/changeset/129184
>
Simon Fraser (smfr)
Comment 16
2012-09-20 20:52:29 PDT
This seems to have caused a webkitpy failure:
http://build.webkit.org/builders/Apple%20MountainLion%20%28Leaks%29/builds/131/steps/webkitpy-test/logs/stdio
Simon Fraser (smfr)
Comment 17
2012-09-20 21:00:04 PDT
Commented out the failing unit test in
http://trac.webkit.org/changeset/129188
Dirk Pranke
Comment 18
2012-09-21 12:23:05 PDT
(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!
Dirk Pranke
Comment 19
2012-09-21 12:34:57 PDT
Test fixed in
http://trac.webkit.org/changeset/129245
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug