| Summary: | new-run-webkit-tests: make the retry step more explicit | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tor Arne Vestbø <vestbo> | ||||
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | abarth, dpranke, eric, ojan | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Other | ||||||
| OS: | OS X 10.5 | ||||||
| Attachments: |
|
||||||
|
Description
Tor Arne Vestbø
2010-04-14 14:06:37 PDT
Created attachment 53366 [details]
Patch
Comment on attachment 53366 [details]
Patch
Did you mean to change it from log.debug to log.info?
(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 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.
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. 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 on attachment 53366 [details]
Patch
ok.
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 Committed r57850: <http://trac.webkit.org/changeset/57850> patch landed (had to change the spelling of the name; apologies). (In reply to comment #10) > patch landed (had to change the spelling of the name; apologies). Thanks, np :) |