Bug 201253 – EWS bubbles should indicate builder vs tester

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Aakash Jain]

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.

[Aakash Jain]

rdar://problem/54674856

[Aakash Jain]

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.

[Aakash Jain]

Created attachment 377515 [details]
Patch

[Aakash Jain]

(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/

[Jonathan Bedard]

(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.

[Jonathan Bedard]

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?

[Simon Fraser (smfr)]

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.

[Simon Fraser (smfr)]

Created attachment 377531 [details]
Empty space on this page

[Alexey Proskuryakov]

True, but the main concern is the review page.

[Simon Fraser (smfr)]

(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.

[Alexey Proskuryakov]

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.

[Aakash Jain]

(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.

[Aakash Jain]

Created attachment 377972 [details]
Patch

[Jonathan Bedard]

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.

[Aakash Jain]

Created attachment 377985 [details]
Patch

[Aakash Jain]

> Nit: I don't think the extra 'icon' variable is helpful here.
Removed in updated patch.

Also fixed the unit-test.

[Aakash Jain]

Committed r249483: <https://trac.webkit.org/changeset/249483>

[Aakash Jain]

Deployed this change in production.

[Aakash Jain]

Note that the review/detail page might show these symbols as well if the browser loads the cached version of the page.