Bug 203411

Summary: [EWS] status bubble tooltip should display failing layout tests names rather than generic 'x failures'
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: ASSIGNED    
Severity: Normal CC: aakash_jain, ap, honamluxurychef, jbedard, ryanhaddad, webkit-bot-watchers-bugzilla, wilander
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=209912
Attachments:
Description Flags
Patch
aakash_jain: review?
Screenshot - tooltip before this change
none
Screenshot - tooltip after this change none

Aakash Jain
Reported 2019-10-25 07:24:29 PDT
Status bubble tooltip message should display failing layout tests names rather than generic 'x failures'. For e.g.: in https://ews-build.webkit.org/#/builders/24/builds/2573, the layout-tests step message simply says: '4 failures 7 flakes', similarly the re-run-layout-tests step message says: '5 failures 4 flakes'. Similarly run-layout-tests-without-patch step message says: '5 failures 7 flakes'. These messages are also displayed in status-bubble tooltip, and aren't very informative. They don't tell the patch author which tests failed. Even after clicking the status-bubble and navigating to the buildbot page, the failures names aren't displayed. One more click is required on 'view layout test results' to look at the failures. If the names of the failing test are displayed in the tooltip itself, patch author might be able to make an estimate if the failing tests might be related to his/her patch or completely unrelated flaky failures. Therefore, we should display the exact test names of the failing tests rather than generic 'x failures' message. Also flakes information might be useful in build.webkit.org, but not useful for EWS, patch author doesn't really care about flakiness unrelated to his/her patch in EWS. Also, layout-test step shouldn't be marked as WARNING due to flakes. EWS purpose is to tell whether the patch is good or bad, displaying WARNING status due to flakiness completely unrelated to the tests isn't the right UX for EWS.
Attachments
Patch (11.72 KB, patch)
2019-10-25 07:45 PDT, Aakash Jain 		aakash_jain: review?
DetailsFormatted DiffDiff
Screenshot - tooltip before this change (110.06 KB, image/png)
2019-10-25 07:57 PDT, Aakash Jain 		no flags
Details
Screenshot - tooltip after this change (295.08 KB, image/png)
2019-10-25 07:59 PDT, Aakash Jain 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Aakash Jain
Comment 1 2019-10-25 07:26:15 PDT
This change will also help bot-watchers to easily notice the flaky tests in a given build, by just looking at the buildbot build page.
Aakash Jain
Comment 2 2019-10-25 07:45:08 PDT
Created attachment 381916 [details] Patch
Aakash Jain
Comment 3 2019-10-25 07:57:11 PDT
Created attachment 381921 [details] Screenshot - tooltip before this change
Aakash Jain
Comment 4 2019-10-25 07:59:59 PDT
Created attachment 381922 [details] Screenshot - tooltip after this change Screenshot of tooltip from another build after this change.
Jonathan Bedard
Comment 5 2019-10-25 08:04:39 PDT
Comment on attachment 381916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381916&action=review > Tools/BuildSlaveSupport/ews-build/steps.py:-1030 > - This seems like a pretty big change, actually....I'm not necessarily opposed to it, but the idea of removing flakiness warnings from EWS might be unpopular, have we had any feedback about those warnings from engineers? > Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1167 > + self.expectOutcome(result=SUCCESS, state_string='Passed layout tests') Does this mean that when a test 'flakes', we say it passed now?
Aakash Jain
Comment 7 2019-10-25 08:34:25 PDT
Comment on attachment 381916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381916&action=review >> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1167 >> + self.expectOutcome(result=SUCCESS, state_string='Passed layout tests') > > Does this mean that when a test 'flakes', we say it passed now? Yes. Note that even currently when layout-test run find a flaky failure, the status-bubble is still green, the Buildbot page says: "Passed layout tests". It is just the build-step which we are changing, not the overall status. e.g.: In https://ews-build.webkit.org/#/builders/17/builds/4699 layout-tests says '4 flakes', but the build and corresponding status-bubble in https://bugs.webkit.org/show_bug.cgi?id=202838 is green. Also, note that some flakes are found in almost every layout-test run, e.g.: https://ews-build.webkit.org/#/builders/17/builds/4604, https://ews-build.webkit.org/#/builders/17/builds/4602, https://ews-build.webkit.org/#/builders/24/builds/24, https://ews-build.webkit.org/#/builders/14/builds/1603, https://ews-build.webkit.org/#/builders/15/builds/598 So until, we have less flakiness, this isn't actionable, and seems like just noise.
Jonathan Bedard
Comment 8 2020-04-02 15:06:21 PDT
I don't like that we're separating test names with commas, that is not very readable. Is there a way to insert newlines?
Note You need to log in before you can comment on or make changes to this bug.