|Summary:||EWS bubbles should indicate builder vs tester|
|Product:||WebKit||Reporter:||Aakash Jain <aakash_jain>|
|Component:||Tools / Tests||Assignee:||Aakash Jain <aakash_jain>|
|Severity:||Normal||CC:||aakash_jain, ap, jbedard, ryanhaddad, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer|
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 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 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 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 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.