Bug 201253 - EWS bubbles should indicate builder vs tester
Summary: EWS bubbles should indicate builder vs tester
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-28 17:12 PDT by Aakash Jain
Modified: 2020-01-26 06:25 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.93 KB, patch)
2019-08-28 17:24 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff
Empty space on this page (567.07 KB, image/png)
2019-08-28 17:50 PDT, Simon Fraser (smfr)
no flags Details
Patch (14.92 KB, patch)
2019-09-04 05:14 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff
Patch (17.20 KB, patch)
2019-09-04 09:49 PDT, Aakash Jain
jbedard: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2019-08-28 17:12:56 PDT
It would be nice to have the EWS bubbles indicate whether the bubble is for builder or for tester or both. 

We can use unicode symbols like ๐Ÿ›  for builder and ๐Ÿงช for tester queues.
Comment 1 Aakash Jain 2019-08-28 17:13:14 PDT
rdar://problem/54674856
Comment 2 Aakash Jain 2019-08-28 17:23:30 PDT
We should add these symbols to the tool-tip of the bubbles. Adding these symbols to the bubble itself would significantly increase the width of the bubbles and bubbles won't fit on one line anymore.
Comment 3 Aakash Jain 2019-08-28 17:24:13 PDT
Created attachment 377515 [details]
Patch
Comment 4 Aakash Jain 2019-08-28 17:28:55 PDT
(In reply to Aakash Jain from comment #3)
> Created attachment 377515 [details]
> Patch
Deployed on UAT instance. 

For most of the Apple queues, it's easy to determine builder vs tester just by the name of the queue. For other queues where the queue name doesn't contain builder or tester, we determine based on build-steps.

e.g.:
https://ews.webkit-uat.org/status-bubble/373911/
https://ews.webkit-uat.org/status-bubble/373459/
https://ews.webkit-uat.org/status-bubble/372833/
Comment 5 Jonathan Bedard 2019-08-28 17:36:22 PDT
(In reply to Aakash Jain from comment #2)
> We should add these symbols to the tool-tip of the bubbles. Adding these
> symbols to the bubble itself would significantly increase the width of the
> bubbles and bubbles won't fit on one line anymore.

Would it Do we have a mandate to fit these bubbles in one line? Given how slow the tooltips are, I would personally find the characters more useful in the bubbles.
Comment 6 Jonathan Bedard 2019-08-28 17:39:30 PDT
Comment on attachment 377515 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377515&action=review

> Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:56
> +    BUILD_STEPS = ['compiled webkit']

Any reason this isn't just: ['compiled', 'build'] instead of being specific to WebKit?

> Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:57
> +    TEST_STEPS = ['api-tests', 'bindings-tests', 'layout-tests', 'webkitperl-tests', 'webkitpy tests']

Any reason this isn't just 'tests' instead of being specific to test suites?
Comment 7 Simon Fraser (smfr) 2019-08-28 17:48:34 PDT
I don't know why we're so worried about bubble space. The whole bug page is so wasteful of space, we could easily redesign to allocate more space to EWS bubbles.
Comment 8 Simon Fraser (smfr) 2019-08-28 17:50:42 PDT
Created attachment 377531 [details]
Empty space on this page
Comment 9 Alexey Proskuryakov 2019-08-28 20:11:34 PDT
True, but the main concern is the review page.
Comment 10 Simon Fraser (smfr) 2019-08-29 12:50:51 PDT
(In reply to Alexey Proskuryakov from comment #9)
> True, but the main concern is the review page.

I rarely consult the bubbles in the review page.
Comment 11 Alexey Proskuryakov 2019-08-29 14:03:51 PDT
How else do you know what EWS said as a reviewer? I think that bubbles on the review page are actually more important, as that's what the reviewer sees.
Comment 12 Aakash Jain 2019-09-03 16:10:01 PDT
(In reply to Simon Fraser (smfr) from comment #7)
> I don't know why we're so worried about bubble space.
ok, let's add these symbols/icons to the bubbles itself.

(In reply to Alexey Proskuryakov from comment #9)
> True, but the main concern is the review page.
We can display these symbols/icons only for the bugs page, not for review page. That should solve both the concerns.

I would need to re-write the logic. Since we would be displaying the symbols in the bubbles, we should display them all the time and irrespective of the build status (rather than trying to determine dynamically from the build, which the current patch tries to do). We would need a static mapping.
Comment 13 Aakash Jain 2019-09-04 05:14:58 PDT
Created attachment 377972 [details]
Patch
Comment 14 Jonathan Bedard 2019-09-04 09:06:36 PDT
Comment on attachment 377972 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=377972&action=review

> Tools/BuildSlaveSupport/ews-app/ews/common/buildbot.py:99
> +            Buildbot.icons_for_queues_mapping[shortname] = icon

Nit: I don't think the extra 'icon' variable is helpful here.
Comment 15 Aakash Jain 2019-09-04 09:49:07 PDT
Created attachment 377985 [details]
Patch
Comment 16 Aakash Jain 2019-09-04 09:49:53 PDT
> Nit: I don't think the extra 'icon' variable is helpful here.
Removed in updated patch.

Also fixed the unit-test.
Comment 17 Aakash Jain 2019-09-04 10:27:20 PDT
Committed r249483: <https://trac.webkit.org/changeset/249483>
Comment 18 Aakash Jain 2019-09-04 10:31:05 PDT
Deployed this change in production.
Comment 19 Aakash Jain 2019-09-04 10:33:51 PDT
Note that the review/detail page might show these symbols as well if the browser loads the cached version of the page.