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

Mihai Parparita
Reported 2010-12-15 18:41:04 PST
Add --exit-after-n-failures/crashes to NRWT
Attachments
Patch (11.41 KB, patch)
2010-12-15 18:42 PST, Mihai Parparita
no flags
Patch (12.19 KB, patch)
2010-12-16 10:52 PST, Mihai Parparita
no flags
Mihai Parparita
Comment 1 2010-12-15 18:42:15 PST
Eric Seidel (no email)
Comment 2 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?
Mihai Parparita
Comment 3 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?
Eric Seidel (no email)
Comment 4 2010-12-15 19:05:33 PST
Huh? What's the difference between -failures and blah-blah-blah-timeouts? Looks like ojan would know.
Ojan Vafai
Comment 5 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.
Ojan Vafai
Comment 6 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.
Eric Seidel (no email)
Comment 7 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.
Ojan Vafai
Comment 8 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
Mihai Parparita
Comment 9 2010-12-16 10:52:53 PST
Mihai Parparita
Comment 10 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.
Dirk Pranke
Comment 11 2010-12-16 11:39:26 PST
Comment on attachment 76787 [details] Patch Change looks good to me. Thanks for doing this!
Dirk Pranke
Comment 12 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.
Eric Seidel (no email)
Comment 13 2010-12-16 13:55:00 PST
Comment on attachment 76787 [details] Patch OK.
WebKit Commit Bot
Comment 14 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
Eric Seidel (no email)
Comment 15 2010-12-16 14:00:27 PST
Comment on attachment 76787 [details] Patch Sorry, I'll go look at the bot.
WebKit Commit Bot
Comment 16 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
Eric Seidel (no email)
Comment 17 2010-12-16 14:08:31 PST
OH. It's nto the bot. THe patch simply fails to apply. :)
Eric Seidel (no email)
Comment 18 2010-12-16 14:11:43 PST
It also appears that git.webkit.org is down, which might be related?
Eric Seidel (no email)
Comment 19 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!
WebKit Commit Bot
Comment 20 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.
WebKit Commit Bot
Comment 21 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>
WebKit Commit Bot
Comment 22 2010-12-16 18:13:38 PST
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.