Bug 51160

Summary: Add --exit-after-n-failures/crashes to NRWT
Product: WebKit Reporter: Mihai Parparita <mihaip>
Component: Tools / TestsAssignee: Mihai Parparita <mihaip>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dpranke, eric, mrobinson, ojan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Description Mihai Parparita 2010-12-15 18:41:04 PST
Add --exit-after-n-failures/crashes to NRWT
Comment 1 Mihai Parparita 2010-12-15 18:42:15 PST
Created attachment 76726 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-12-15 18:49:07 PST
Comment on attachment 76726 [details]
Patch

Looks OK.  I'm sad that there are two of these. I guess I didn't know the "crashes" varient had been added.  I wonder what itch that is scratching?
Comment 3 Mihai Parparita 2010-12-15 18:57:15 PST
The ChangeLog from http://trac.webkit.org/changeset/62728 has the motivation (Ojan, can you elaborate?).

Additionally, it looks like --exit-after-n-crashes was changed to --exit-after-n-crashes-or-timeouts with http://trac.webkit.org/changeset/62732. I guess I should mirror that here too?
Comment 4 Eric Seidel (no email) 2010-12-15 19:05:33 PST
Huh? What's the difference between -failures and blah-blah-blah-timeouts?

Looks like ojan would know.
Comment 5 Ojan Vafai 2010-12-15 19:50:12 PST
exit-after-n-crashes-or-timeouts lets the bots run with a ton of failures. This is important for being able to land a change that needs a ton of new platform-specific expectations. You need the bot to run the test in order to grab the new results.

We exit after n crashes/timeouts because those are the ones that are really slow and thus cause the bot to take forever to cycle. But we don't care about other failure types because they don't cause cycle time to be slow.

All of this is in the ChangeLog descriptions of those two patches.

exit-after-n-failures is just there for the commit-queue I believe since it wants to bail ASAP.
Comment 6 Ojan Vafai 2010-12-15 19:50:30 PST
(In reply to comment #3)
> Additionally, it looks like --exit-after-n-crashes was changed to --exit-after-n-crashes-or-timeouts with http://trac.webkit.org/changeset/62732. I guess I should mirror that here too?

Yes.
Comment 7 Eric Seidel (no email) 2010-12-15 19:55:49 PST
I wrote --exit-after-N-failures initially to get around waiting for a patch which caused every test to crash.

It's true that the CQ likes exiting after any failure, but if every other use case is for the --exit-blah-blah-timeouts case, then maybe that should be the only one.

Or maybe the "exit-after-timeouts" whatever thing should just be built in to both harnesses.   I would want NRWT to bail after 30 crashes or timeouts regardless, I would think.
Comment 8 Ojan Vafai 2010-12-15 20:51:53 PST
(In reply to comment #7)
> It's true that the CQ likes exiting after any failure, but if every other use case is for the --exit-blah-blah-timeouts case, then maybe that should be the only one.

Fine with me.

> Or maybe the "exit-after-timeouts" whatever thing should just be built in to both harnesses.   I would want NRWT to bail after 30 crashes or timeouts regardless, I would think.

I support getting rid of the commandline option and just always having it run. Although, in the case of chromium tests, we'd want to bail after 30 unexpected crash/timeouts
Comment 9 Mihai Parparita 2010-12-16 10:52:53 PST
Created attachment 76787 [details]
Patch
Comment 10 Mihai Parparita 2010-12-16 10:54:25 PST
I found --exit-after-n-failures useful when used in combinations with --iterations for tracking down flaky tests (--iterations 100 --exit-after-n-failures 1 stops on the first failure). Adding --iterations support to NRWT is next on my todo list.

I've updated the patch to support --exit-after-n-crashes-or-timeouts.
Comment 11 Dirk Pranke 2010-12-16 11:39:26 PST
Comment on attachment 76787 [details]
Patch

Change looks good to me. Thanks for doing this!
Comment 12 Dirk Pranke 2010-12-16 11:41:01 PST
I have no real opinions on any of these flags, by the way. Whatever you guys decide is fine by me.
Comment 13 Eric Seidel (no email) 2010-12-16 13:55:00 PST
Comment on attachment 76787 [details]
Patch

OK.
Comment 14 WebKit Commit Bot 2010-12-16 13:58:30 PST
Comment on attachment 76787 [details]
Patch

Rejecting attachment 76787 [details] from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--non-interactive', 76787]" exit_code: 2
Last 500 characters of output:
ailed to merge in the changes.
Patch failed at 0001 2010-12-16  Yury Semikhatsky  <yurys@chromium.org>

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at WebKitTools/Scripts/update-webkit line 132.

Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2

Full output: http://queues.webkit.org/results/7176074
Comment 15 Eric Seidel (no email) 2010-12-16 14:00:27 PST
Comment on attachment 76787 [details]
Patch

Sorry, I'll go look at the bot.
Comment 16 WebKit Commit Bot 2010-12-16 14:02:07 PST
Comment on attachment 76787 [details]
Patch

Rejecting attachment 76787 [details] from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--non-interactive', 76787]" exit_code: 2
Last 500 characters of output:
ailed to merge in the changes.
Patch failed at 0001 2010-12-16  Yury Semikhatsky  <yurys@chromium.org>

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at WebKitTools/Scripts/update-webkit line 132.

Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2

Full output: http://queues.webkit.org/results/7140100
Comment 17 Eric Seidel (no email) 2010-12-16 14:08:31 PST
OH.  It's nto the bot.  THe patch simply fails to apply. :)
Comment 18 Eric Seidel (no email) 2010-12-16 14:11:43 PST
It also appears that git.webkit.org is down, which might be related?
Comment 19 Eric Seidel (no email) 2010-12-16 14:20:24 PST
Comment on attachment 76787 [details]
Patch

No, the bot itself was wedged.  I'm fixing it now.  Sorry!
Comment 20 WebKit Commit Bot 2010-12-16 16:06:31 PST
The commit-queue encountered the following flaky tests while processing attachment 76787 [details]:

fast/workers/storage/use-same-database-in-page-and-workers.html bug 50995 (author: dumi@chromium.org)
java/argument-to-object-type.html bug 51102 (author: ap@webkit.org)
The commit-queue is continuing to process your patch.
Comment 21 WebKit Commit Bot 2010-12-16 18:13:30 PST
Comment on attachment 76787 [details]
Patch

Clearing flags on attachment: 76787

Committed r74223: <http://trac.webkit.org/changeset/74223>
Comment 22 WebKit Commit Bot 2010-12-16 18:13:38 PST
All reviewed patches have been landed.  Closing bug.