Summary: | new-run-webkit-tests: Bot master should print useful information on waterfall/console for nrwt | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||||||||||||||||||
Component: | Tools / Tests | Assignee: | Ryosuke Niwa <rniwa> | ||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||
Severity: | Normal | CC: | abarth, aroben, dpranke, eric, ojan, sam, tony, wsiegrist | ||||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||||||||
Bug Blocks: | 34984 | ||||||||||||||||||||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2011-06-06 19:27:45 PDT
Created attachment 96179 [details]
screenshot 1
Created attachment 96180 [details]
screenshot 2
There's too much text there. Can we convey the information with less text? (In reply to comment #3) > There's too much text there. Can we convey the information with less text? Yeah, I agree. Do you have a suggestion as to how we do it? Maybe I can get rid of " mismatch ". I don't know how I can shorten "Expected to fail, but passed" and "Expected to timeout, but passed" Created attachment 96181 [details]
fixes the bug
Created attachment 96198 [details]
screenshot 1'
Created attachment 96199 [details]
screenshot 2'
Created attachment 96200 [details]
shortened text
*** Bug 59894 has been marked as a duplicate of this bug. *** This still seems like too much text and the wrapping makes it hard to read in the narrow view. How about just totals like: flaky: # failed: # And if there are no failed tests: unexpected pass: # flaky: # Alternately, we should just match the text of the other bots and work from there (i.e., work on improving all the bots output). Comment on attachment 96200 [details]
shortened text
I don't think we should print out what kind of failure/flaky it is. Just a sum number for each of the following:
# failures
# flaky
# unexpected passes
Printing out more is too much information to make sense of. Also, perhaps as a separate patch, I'd really like to see us printout the first 10 failures as well like we do on the build.chromium.org waterfall. That really helps a lot in identifying when a test started failing.
(In reply to comment #11) > This still seems like too much text and the wrapping makes it hard to read in the narrow view. > > How about just totals like: > > flaky: # > failed: # I agree. We should shorten the text. But I'd like the number of failures be left-aligned. It makes a huge difference in readability. (In reply to comment #12) > (From update of attachment 96200 [details]) > I don't think we should print out what kind of failure/flaky it is. Just a sum number for each of the following: > # failures > # flaky > # unexpected passes Agreed. > Printing out more is too much information to make sense of. Also, perhaps as a separate patch, I'd really like to see us printout the first 10 failures as well like we do on the build.chromium.org waterfall. That really helps a lot in identifying when a test started failing. I personally find them a pure noise on build.chromium.org bots. Maybe 1-2 tests would be fine but when there are more than 3+ tests, it clutters the view (epecially when test names are long) and I can't make sense of it. Since we'd still have to look at diffs in order to determine what kind of failures they are, I don't see it as a critical feature at the moment. BTW, it'll be nice for Chromium port's results.html to have a link to flakiness dashboard. (In reply to comment #13) > > Printing out more is too much information to make sense of. Also, perhaps as a separate patch, I'd really like to see us printout the first 10 failures as well like we do on the build.chromium.org waterfall. That really helps a lot in identifying when a test started failing. > > I personally find them a pure noise on build.chromium.org bots. Maybe 1-2 tests would be fine but when there are more than 3+ tests, it clutters the view (epecially when test names are long) and I can't make sense of it. I'd side with Ojan on this one. On a side note, I am working on splitting out all of the "useful" output from new-run-webkit-tests from the debug output, so the stdio output should become a lot less noisy soon. When we list the failures it makes it super easy to tell from a quick glance at the waterfall (perhaps with some find-in-page) when a test started failing. Without that you either need to look at the flakiness dashboard or actually look at the stdio for each run. (In reply to comment #15) > When we list the failures it makes it super easy to tell from a quick glance at the waterfall (perhaps with some find-in-page) when a test started failing. Without that you either need to look at the flakiness dashboard or actually look at the stdio for each run. Okay, I can see that people have different opinions on this. It'd be nice if this was configurable (the number of tests to show, etc...). But if we're making this change for new-run-webkit-tests, then we should be making the same change for old-run-webkit-tests because I don't like nrwt and orwt having different look & feel. Created attachment 96289 [details]
screenshot 1''
Created attachment 96290 [details]
screenshot 2''
Created attachment 96291 [details]
shortened text further
Comment on attachment 96291 [details] shortened text further View in context: https://bugs.webkit.org/attachment.cgi?id=96291&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:287 > + match = expressions[name].match(line) Nit: Maybe use search() instead of match()? search() matches anywhere in the string and match() only matches the beginning of the string. > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:292 > + if name not in testFailures: > + testFailures[name] = 0 > + testFailures[name] += int(match.group(1)) This could be testFailures[name] = testFailures.get(name, 0) + int(match.group(1)) > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:313 > + return SUCCESS Did you mean to return result here? I think the text is good enough as is, but if you were still looking to compress further, here are some suggestions: 1) omit "tests" ... "14 failed" is good enough 2) s/were flaky/flaky ... "10 flaky" is also good enough 3) you could omit both the flaky and unexpected pass numbers if you had actual failures. On the chromium bots, neither of those sets of results would produce a failed test step, so you be okay with skipping them. 4) I attempted to come up with something shorter than "passed unexpectedly", and failed. Perhaps we could coin a word like "unfailed" :) Personally, I'd probably make the changes in (1) and (2) and not the others. Up to you, though. (In reply to comment #21) > I think the text is good enough as is, but if you were still looking to compress further, here are some suggestions: > > 1) omit "tests" ... "14 failed" is good enough > > 2) s/were flaky/flaky ... "10 flaky" is also good enough These also have the benefit of not having to worry about tenses. '1 tests failed' and '1 tests were flaky' is grammatically incorrect. (In reply to comment #21) > 1) omit "tests" ... "14 failed" is good enough > > 2) s/were flaky/flaky ... "10 flaky" is also good enough Yeah, I experimented these two. Thought 14 failed / 14 passed sounded funny but so does "1 tests failed" so let's do this. > 3) you could omit both the flaky and unexpected pass numbers if you had actual failures. On the chromium bots, neither of those sets of results would produce a failed test step, so you be okay with skipping them. We definitely can do that in a followup patch if the current version proves to be too noisy. > 4) I attempted to come up with something shorter than "passed unexpectedly", and failed. Perhaps we could coin a word like "unfailed" :) Yeah, I tried and failed as well. Created attachment 96324 [details]
screenshot 1'''
Created attachment 96325 [details]
screenshot 2'''
Created attachment 96329 [details]
Patch
Committed r88310: <http://trac.webkit.org/changeset/88310> wms, aroben: could either one of you reconfig the master to pick up r88310? Master updated. (In reply to comment #29) > Master updated. Thanks! (In reply to comment #12) > Also, perhaps as a separate patch, I'd really like to see us printout the first 10 failures as well like we do on the build.chromium.org waterfall. That really helps a lot in identifying when a test started failing. You might be interested in bug 59521 and bug 61877. |