Bug 176332 - EWS should report when a step succeeds
Summary: EWS should report when a step succeeds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-09-04 13:24 PDT by Aakash Jain
Modified: 2017-09-08 11:24 PDT (History)
7 users (show)

See Also:


Attachments
Proposed patch (4.41 KB, patch)
2017-09-04 14:22 PDT, Aakash Jain
sam: review+
Details | Formatted Diff | Diff
Screenshot_bubbles_during_processing (144.69 KB, image/png)
2017-09-04 21:38 PDT, Aakash Jain
no flags Details
Screenshot_bubbles_after_processing (110.47 KB, image/png)
2017-09-04 21:39 PDT, Aakash Jain
no flags Details
Patch with updated ChangeLog (4.89 KB, patch)
2017-09-05 08:30 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2017-09-04 13:24:19 PDT
Most of the EWS report only if a step fails, but do not report when a step succeed. For e.g.: If a build succeeds, EWS does not report it. Information about a step passing is sometimes very valuable, especially when waiting for a patch to complete processing. Sometimes, patch authors know that their changes are not covered by any tests. It makes little sense to wait for tests to run in this case. See <rdar://problem/25224607>

StyleQueue and CommitQueue do report both pass and fail status of a step. For e.g.: See the difference in logging between CommitQueue and ios-ews: 
https://webkit-queues.webkit.org/patch/285756/ios-ews
https://webkit-queues.webkit.org/patch/285756/commit-queue

All the Queues should report passing status of steps as well.
Comment 1 Aakash Jain 2017-09-04 14:22:10 PDT
After the change to have individual web-pages for each EWS queue (https://bugs.webkit.org/show_bug.cgi?id=174477), we can display more information about the patch without hurting the UI.
Comment 2 Aakash Jain 2017-09-04 14:22:33 PDT
Created attachment 319860 [details]
Proposed patch
Comment 3 Sam Weinig 2017-09-04 16:26:49 PDT
Comment on attachment 319860 [details]
Proposed patch

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

> Tools/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * Scripts/webkitpy/tool/commands/earlywarningsystem.py:

Nice. The ChangeLog could use a bit more of a 'why' description.  You have a pretty good one in the first comment of the bug which should take you most of the way there!
Comment 4 Alexey Proskuryakov 2017-09-04 17:05:25 PDT
Does this change also propagate to the popover? How does it look after the build succeeds?
Comment 5 Aakash Jain 2017-09-04 21:38:41 PDT
Created attachment 319874 [details]
Screenshot_bubbles_during_processing
Comment 6 Aakash Jain 2017-09-04 21:39:20 PDT
Created attachment 319875 [details]
Screenshot_bubbles_after_processing
Comment 7 Aakash Jain 2017-09-04 21:43:33 PDT
(In reply to Alexey Proskuryakov from comment #4)
> Does this change also propagate to the popover?

Yes, the pop-over does show these messages in "Recent messages" section. That should help developers to just hover over the bubbles and get more insight into what's going on.

>  How does it look after the build succeeds?
No change from earlier. It shows Pass status.

Screenshots attached from testing instance.
Comment 8 Aakash Jain 2017-09-05 08:30:16 PDT
Created attachment 319896 [details]
Patch with updated ChangeLog
Comment 9 WebKit Commit Bot 2017-09-05 16:54:21 PDT
Comment on attachment 319896 [details]
Patch with updated ChangeLog

Clearing flags on attachment: 319896

Committed r221649: <http://trac.webkit.org/changeset/221649>
Comment 10 WebKit Commit Bot 2017-09-05 16:54:22 PDT
All reviewed patches have been landed.  Closing bug.