Bug 196584

Summary:

[ews-app] Status bubble should not display skipped buildbot steps

Product:

WebKit

Reporter:

Aakash Jain <aakash_jain>

Component:

Tools / Tests

Assignee:

Aakash Jain <aakash_jain>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aakash_jain, ap, commit-queue, dewei_zhu, jbedard, lforschler, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=196550
https://bugs.webkit.org/show_bug.cgi?id=201308

Attachments:

Description	Flags
Screenshot without the fix	none
Patch	none

Aakash Jain

Reported 2019-04-03 17:49:32 PDT

Created attachment 366681 [details] Screenshot without the fix [ews-app] Status bubble should not display skipped buildbot steps. Since these steps were not run (in buildbot build), these shouldn't be displayed in the status-bubble hover-over message. For example in attached screenshot 'Unapplied patch (skipped)' and 'Compile Webkit (skipped)' should not be shown in status-bubble, since these steps were not run by Buildbot.

Attachments
Screenshot without the fix (77.29 KB, image/png) 2019-04-03 17:49 PDT, Aakash Jain	no flags	Details
Patch (1.33 KB, patch) 2019-04-03 17:52 PDT, Aakash Jain	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Aakash Jain

Comment 1 2019-04-03 17:52:43 PDT

Created attachment 366682 [details] Patch

Jonathan Bedard

Comment 2 2019-04-04 09:05:18 PDT

Comment on attachment 366682 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366682&action=review > Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:44 > + STEPS_TO_HIDE = ['Killed old processes', 'Configured build', '^OS:.*Xcode:', '(skipped)'] ...I'm not sure this is actually what we want. It makes sense if some clean-up steps get squashed, but what happens if we skip something like API tests or layout tests? Would that even be possible?

Aakash Jain

Comment 3 2019-04-04 12:35:20 PDT

> ...I'm not sure this is actually what we want. It makes sense if some > clean-up steps get squashed, but what happens if we skip something like API > tests or layout tests? Would that even be possible? I don't see a scenario when buildbot would skip the step (based on doStepIf) and we want to intentionally show it in status-bubble. We have '(skipped)' in the step state_string when buildbot skips the step (usually based on doStepIf). For e.g.: in this case we have step to Unapply patch, this step is present in almost all builds and is usually skipped, it is run based on doStepIf (when the compilation fails). Displaying the status 'Unapplied Patch (skipped)' would be pretty confusing for the user. We don't have doStepIf in crucial steps like API tests or layout tests, so these shouldn't be skipped.

WebKit Commit Bot

Comment 4 2019-04-04 13:38:56 PDT

Comment on attachment 366682 [details] Patch Clearing flags on attachment: 366682 Committed r243897: <https://trac.webkit.org/changeset/243897>

WebKit Commit Bot

Comment 5 2019-04-04 13:38:58 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 6 2019-04-04 13:39:24 PDT

<rdar://problem/49620520>

Note You need to log in before you can comment on or make changes to this bug.