Bug 37606 - new-run-webkit-tests: make the retry step more explicit
Summary: new-run-webkit-tests: make the retry step more explicit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-14 14:06 PDT by Tor Arne Vestbø
Modified: 2010-04-23 03:12 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.01 KB, patch)
2010-04-14 14:09 PDT, Tor Arne Vestbø
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tor Arne Vestbø 2010-04-14 14:06:37 PDT
new-run-webkit-tests: make the retry step more explicit
Comment 1 Tor Arne Vestbø 2010-04-14 14:09:24 PDT
Created attachment 53366 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-04-14 14:16:04 PDT
Comment on attachment 53366 [details]
Patch

Did you mean to change it from log.debug to log.info?
Comment 3 Tor Arne Vestbø 2010-04-14 14:28:30 PDT
(In reply to comment #2)
> (From update of attachment 53366 [details])
> Did you mean to change it from log.debug to log.info?

Yes. Retries is on by default, so if you run without any flags (no --verbose), we will do retries but you won't see at which point we switched to retry mode. With info this line is printed even without --verbose, so the user sees what's going on (not just the meter changing from "Testing" to "Retrying").
Comment 4 Adam Barth 2010-04-17 10:59:46 PDT
Comment on attachment 53366 [details]
Patch

I think we want to keep debug-level logging.  The design is to only print one line of status, not spew to the console in non-verbose mode.
Comment 5 Dirk Pranke 2010-04-17 11:02:23 PDT
I'm not a reviewer, so I can't R+ this, but I actually really like this change. I have also thought that it's a bit confusing to know when things are being retried, and I would like some visual separation between the initial set of failures and the retried set.
Comment 6 Dirk Pranke 2010-04-17 11:04:12 PDT
Oh, additionally, the design is to only print one line of status, as you say, but it is okay to print things when something goes wrong (warnings, errors, etc). The fact that you have to retry thing qualifies in my book. The retry is not quite a warning, but it's a little more severe than a transient status message.
Comment 7 Adam Barth 2010-04-17 11:12:00 PDT
Comment on attachment 53366 [details]
Patch

ok.
Comment 8 Dirk Pranke 2010-04-18 18:12:50 PDT
This patch is good, but if you felt like it, I would actually tweak it slightly.

The current code supports more than one retry, but in practice we've never actually needed this. I suggest we change the counter to a boolean.

Also, we should add a "--no-retry-failures" flag to not retry the failures at all.

Let me know if you want to make these changes, or if you want to land the patch as is and let me change things, or if you want me to make the changes and land the revised patch
Comment 9 Dirk Pranke 2010-04-19 16:05:54 PDT
Committed r57850: <http://trac.webkit.org/changeset/57850>
Comment 10 Dirk Pranke 2010-04-19 16:06:21 PDT
patch landed (had to change the spelling of the name; apologies).
Comment 11 Tor Arne Vestbø 2010-04-23 03:12:02 PDT
(In reply to comment #10)
> patch landed (had to change the spelling of the name; apologies).

Thanks, np :)