WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
112890
NRWT: --force should run tests as if they're expected to pass
https://bugs.webkit.org/show_bug.cgi?id=112890
Summary
NRWT: --force should run tests as if they're expected to pass
Ryosuke Niwa
Reported
2013-03-21 02:26:04 PDT
--force appears to be broken at least on Mac WK1.
Attachments
Modifies the behaviour of nrwt --force switch.
(5.35 KB, patch)
2013-10-21 07:07 PDT
,
Tamas Gergely
no flags
Details
Formatted Diff
Diff
Modifies the behaviour of nrwt --force switch.
(5.35 KB, patch)
2013-10-31 04:13 PDT
,
Tamas Gergely
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1
2013-03-21 02:38:21 PDT
--force Run all tests, even those marked SKIP in the test list (same as --skipped=ignore)
Ryosuke Niwa
Comment 2
2013-03-21 11:05:46 PDT
(In reply to
comment #1
)
> --force Run all tests, even those marked SKIP in the test list (same as --skipped=ignore)
Hm... maybe it's doing the right thing? What I would expect --force to do is to treat all tests to have Pass expectation.
Dirk Pranke
Comment 3
2013-03-21 12:06:14 PDT
Yup, that's not what it does (nor is it what it's ever done). You're suggesting a reasonable feature addition, though.
Dirk Pranke
Comment 4
2013-03-21 13:52:36 PDT
To clarify, if a test is marked as Skip (only), then when it is run, it will be expected to pass. You're suggesting that --force should do something else, and also change the expectations for tests that are otherwise normally run but expected to fail one way or another?
Ryosuke Niwa
Comment 5
2013-03-21 13:54:20 PDT
(In reply to
comment #4
)
> To clarify, if a test is marked as Skip (only), then when it is run, it will be expected to pass. > > You're suggesting that --force should do something else, and also change the expectations for tests that are otherwise normally run but expected to fail one way or another?
I'm expecting that Failure, ImageOnlyFailure, Crash, etc… test expectations to be ignored, and all tests to be expected to Pass.
Dirk Pranke
Comment 6
2013-03-21 13:55:39 PDT
Ok. I'm not opposed to this idea, but I'm not sure if we should use --force for that. It might confuse people used to the existing behavior (or break scripts using it). On the other hand, it does seem like a reasonable name, and we do have --skipped= for the other behavior.
Ryosuke Niwa
Comment 7
2013-03-21 14:01:46 PDT
(In reply to
comment #6
)
> Ok. I'm not opposed to this idea, but I'm not sure if we should use --force for that. It might confuse people used to the existing behavior (or break scripts using it). > > On the other hand, it does seem like a reasonable name, and we do have --skipped= for the other behavior.
Do you know if there are any tools that depend on this behavior? I feel like the current --force should be renamed to --ignore-skips.
Ryosuke Niwa
Comment 8
2013-03-21 14:02:16 PDT
Ugh… I guess ignoring skips might sound ambiguous. Maybe --run-skipped?
Dirk Pranke
Comment 9
2013-03-21 14:09:20 PDT
--force is currently a synonym for --skipped=ignore . If we can get people to change at all, they should just change to that, rather than adding yet another --run-skipped or --ignore-skipped flag. I do not know if any scripts currently use this. I think Ossy has done something along these lines in the past, so I'm cc'ing him.
Ryosuke Niwa
Comment 10
2013-03-21 14:12:23 PDT
(In reply to
comment #9
)
> --force is currently a synonym for --skipped=ignore . If we can get people to change at all, they should just change to that, rather than adding yet another --run-skipped or --ignore-skipped flag.
Yeah, I agree. We already have waaay too many options in new-run-webkit-tests.
Tamas Gergely
Comment 11
2013-10-21 07:07:19 PDT
Created
attachment 214736
[details]
Modifies the behaviour of nrwt --force switch.
Dirk Pranke
Comment 12
2013-10-21 11:42:37 PDT
The code looks correct. However, this patch appears to ignore much of the discussion in the bug ... are you sure that you want to change the semantics of the existing flag? Insofar as I'm not working on the code in WebKit much these days, I'll let someone else (rniwa?) decide if they want to r+ this or not.
WebKit Commit Bot
Comment 13
2013-10-21 19:16:01 PDT
Comment on
attachment 214736
[details]
Modifies the behaviour of nrwt --force switch. Clearing flags on attachment: 214736 Committed
r157774
: <
http://trac.webkit.org/changeset/157774
>
WebKit Commit Bot
Comment 14
2013-10-21 19:16:04 PDT
All reviewed patches have been landed. Closing bug.
Roger Fong
Comment 15
2013-10-22 11:56:01 PDT
This caused 2 python tests to fail on all platforms:
http://build.webkit.org/builders/Apple%20MountainLion%20Release%20WK2%20%28Tests%29/builds/12800/steps/webkitpy-test/logs/stdio
Csaba Osztrogonác
Comment 16
2013-10-23 01:53:28 PDT
Comment on
attachment 214736
[details]
Modifies the behaviour of nrwt --force switch. View in context:
https://bugs.webkit.org/attachment.cgi?id=214736&action=review
> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:341 > + if options.force and options.skipped not in ('ignore', 'default'): > + _log.warning("--force overrides --skipped=%s" % (options.skipped)) > + options.skipped = 'ignore' > +
It seems the bug is here. After this change --force makes NRWT run [ Failure ] marked tests if they marked as [ Pass ], but it doesn't run [ Skip ] and [ WontFix ] marked tests as they pass. The problem is that options.skipped = 'default' by default, but it should be 'ignore' if options.force is true. Tamás, please run Tools/Scripts/test-webkitpy unit tests next time you touches webkitpy codes. I thought CQ bots run webkitpy tests ... but it seems they don't. I found this: "# FIXME: We should teach the commit-queue and the EWS how to run these tests." in Tools/Scripts/webkitpy/tool/steps/runtests.py
Tamas Gergely
Comment 17
2013-10-31 04:13:21 PDT
Created
attachment 215642
[details]
Modifies the behaviour of nrwt --force switch. Corrected, now --force really implies --skipped=ignore
WebKit Commit Bot
Comment 18
2013-10-31 11:11:22 PDT
Comment on
attachment 215642
[details]
Modifies the behaviour of nrwt --force switch. Clearing flags on attachment: 215642 Committed
r158373
: <
http://trac.webkit.org/changeset/158373
>
WebKit Commit Bot
Comment 19
2013-10-31 11:11:27 PDT
All reviewed patches have been landed. Closing bug.
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