Bug 203418

Summary: [EWS] Status page should show compiler ERRORS
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, bfulgham, jbedard, webkit-bot-watchers-bugzilla, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=215174
https://bugs.webkit.org/show_bug.cgi?id=211344
Attachments:
Description Flags
Current view
none
Patch
none
Patch
jbedard: review+
Screenshot - view after this change none

Description Brent Fulgham 2019-10-25 09:03:12 PDT
The EWS results presented when you click on a failure bubble currently gives you the option of looking at warnings, or looking at the entire log file (which can be tens of thousands of lines).

We should also offer a filtered "errors" view that only shows the compile errors. These are generally the most important things to look at. Warnings are generally not as useful for figuring out why an EWS bot is unhappy.
Comment 1 Brent Fulgham 2019-10-25 09:04:10 PDT
Created attachment 381929 [details]
Current view

Here's a screenshot showing the two options. A much better tool would be a summary of the compiler errors.
Comment 2 Brent Fulgham 2019-10-25 09:06:40 PDT
You might consider passing the output through Dino's old "filter-build-webkit" script, which did a good job of culling out boilerplate noise, and showing only the meaningful output from a compile.
Comment 3 Radar WebKit Bug Importer 2019-10-25 09:07:04 PDT
<rdar://problem/56619679>
Comment 4 Aakash Jain 2019-10-25 11:45:22 PDT
Created attachment 381950 [details]
Patch
Comment 6 Jonathan Bedard 2019-10-25 11:51:27 PDT
Comment on attachment 381950 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/steps.py:747
> +        is_whitespace = self.whitespace_re.search(line) is not None

This temporary variable doesn't seem super useful

> Tools/BuildSlaveSupport/ews-build/steps.py:754
> +            map(self.errorReceived, self.error_context_buffer[:-1])

Why the -1? Couldn't we just map errorReceived to the whole buffer, which would include the line?
Comment 7 Brent Fulgham 2019-10-25 11:55:49 PDT
(In reply to Aakash Jain from comment #4)
> Created attachment 381950 [details]
> Patch

Oh gosh! I'm so happy! :-)
Comment 8 Aakash Jain 2019-10-25 12:50:18 PDT
(In reply to Jonathan Bedard from comment #6)
> Why the -1? Couldn't we just map errorReceived to the whole buffer, which would include the line?
Changed in updated patch.
Comment 9 Aakash Jain 2019-10-25 12:51:06 PDT
Created attachment 381957 [details]
Patch
Comment 11 Brent Fulgham 2019-10-25 13:44:38 PDT
(In reply to Aakash Jain from comment #10)
> Sample runs:
> https://ews-build.webkit-uat.org/#/builders/3/builds/109
> https://ews-build.webkit-uat.org/#/builders/3/builds/110

These look great!
Comment 12 Aakash Jain 2019-10-25 15:07:16 PDT
Committed r251604: <https://trac.webkit.org/changeset/251604>
Comment 13 Aakash Jain 2019-10-26 08:48:40 PDT
Deployed on the production server.
Comment 14 Aakash Jain 2019-10-28 09:16:44 PDT
Created attachment 382076 [details]
Screenshot - view after this change

From https://ews-build.webkit.org/#/builders/16/builds/6054
Comment 15 Aakash Jain 2019-10-28 10:57:15 PDT
*** Bug 201941 has been marked as a duplicate of this bug. ***