Bug 37606 – new-run-webkit-tests: make the retry step more explicit

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tor Arne Vestbø]

new-run-webkit-tests: make the retry step more explicit

[Tor Arne Vestbø]

Created attachment 53366 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 53366 [details]
Patch

Did you mean to change it from log.debug to log.info?

[Tor Arne Vestbø]

(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").

[Adam Barth]

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.

[Dirk Pranke]

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.

[Dirk Pranke]

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.

[Adam Barth]

Comment on attachment 53366 [details]
Patch

ok.

[Dirk Pranke]

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

[Dirk Pranke]

Committed r57850: <http://trac.webkit.org/changeset/57850>

[Dirk Pranke]

patch landed (had to change the spelling of the name; apologies).

[Tor Arne Vestbø]

(In reply to comment #10)
> patch landed (had to change the spelling of the name; apologies).

Thanks, np :)