Bug 207026

Summary: [ews] add build step to set custom build summary
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, jbedard, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 207027    
Attachments:
Description Flags
Patch none

Description Aakash Jain 2020-01-30 18:59:46 PST 
[ews] add build step to set custom build summary based on a property which any previous step can set. 

Currently in order to set custom build summary we are using self.build.buildFinished(build_summary, result) method, but this method also finishes the build immediately whenever it's called. So if a step uses buildFinished(), no further build step can be run.

For example: Layout-test set custom build summary (e.g.: 'Passed layout tests', 'Found 1 pre-existing test failure: xyz'). In case commit-queue (or any factory) run layout-test step, layout-test will call self.build.buildFinished() and commit-queue (or that factory) would not be able to execute any subsequent build-step.

The fix is to avoid calling buildFinished() inside the regular build-steps, instead set a property in those build-steps, and at the end run a specific build-step (e.g.: SetBuildSummary) which would set the build summary based on that property.
Comment 1 Aakash Jain 2020-01-30 19:01:32 PST 
Created attachment 389325 [details]
Patch
Comment 3 Jonathan Bedard 2020-01-31 09:27:51 PST 
Comment on attachment 389325 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/steps.py:2021
> +        self.build.buildFinished([build_summary], self.build.results)

What is self.build.results?
Comment 4 Aakash Jain 2020-01-31 09:35:48 PST 
(In reply to Jonathan Bedard from comment #3)
> What is self.build.results?
results is a variable inside Build class https://github.com/buildbot/buildbot/blob/master/master/buildbot/process/build.py#L77

It stores the current result based on the steps which have executed. 

This I am trying SetBuildSummary to set the same overall build status which Buildbot would have set otherwise, based on the status of previously executed steps.
Comment 5 Jonathan Bedard 2020-01-31 09:40:21 PST 
(In reply to Aakash Jain from comment #4)
> (In reply to Jonathan Bedard from comment #3)
> > What is self.build.results?
> results is a variable inside Build class
> https://github.com/buildbot/buildbot/blob/master/master/buildbot/process/
> build.py#L77
> 
> It stores the current result based on the steps which have executed. 
> 
> This I am trying SetBuildSummary to set the same overall build status which
> Buildbot would have set otherwise, based on the status of previously
> executed steps.

Can we contrive an example where Python 2 fails, but python 3 does not? I think this is right, but that's the only case we haven't tested.
Comment 6 Aakash Jain 2020-01-31 09:44:22 PST 
(In reply to Jonathan Bedard from comment #5)
> Can we contrive an example where Python 2 fails, but python 3 does not? I think this is right, but that's the only case we haven't tested.
Will provide that in https://bugs.webkit.org/show_bug.cgi?id=207027
Comment 7 Aakash Jain 2020-01-31 10:05:10 PST 
Comment on attachment 389325 [details]
Patch

Clearing flags on attachment: 389325

Committed r255506: <https://trac.webkit.org/changeset/255506>
Comment 8 Aakash Jain 2020-01-31 10:05:14 PST 
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2020-01-31 10:06:20 PST 
<rdar://problem/59065405>