Bug 187358 – [ews-build] EWS should unapply the patch and build ToT when patch fails to build

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Aakash Jain]

When a patch fails to build, EWS should unapply that patch and build ToT, in order to confirm that it is not a pre-existing issue.

[Aakash Jain]

Created attachment 344361 [details]
Proposed Patch

Tested in http://ews-build.webkit-uat.org/#/builders/9/builds/22

This depends on patch in https://bugs.webkit.org/show_bug.cgi?id=187357

[EWS Watchlist]

Attachment 344361 [details] did not pass style-queue:


ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:154:  [CompileWebKit.evaluateCommand] Use of super on an old style class  [pylint/E1002] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:466:  [TestCompileWebKitToT.test_success] Passing unexpected keyword argument 'state_string' in function call  [pylint/E1123] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:466:  [TestCompileWebKitToT.test_success] No value passed for parameter 'status_text' in function call  [pylint/E1120] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:481:  [TestCompileWebKitToT.test_failure] Passing unexpected keyword argument 'state_string' in function call  [pylint/E1123] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:481:  [TestCompileWebKitToT.test_failure] No value passed for parameter 'status_text' in function call  [pylint/E1120] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:489:  [TestCompileWebKitToT.test_skip] Passing unexpected keyword argument 'state_string' in function call  [pylint/E1123] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:489:  [TestCompileWebKitToT.test_skip] No value passed for parameter 'status_text' in function call  [pylint/E1120] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/factories.py:66:  [BuildFactory.__init__] Undefined variable 'UnApplyPatchIfRequired'  [pylint/E0602] [5]
Total errors found: 8 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Aakash Jain]

Style queue messages are false positive.

UnApplyPatchIfRequired variable is defined in https://bugs.webkit.org/show_bug.cgi?id=187357

[EWS Watchlist]

Comment on attachment 344361 [details]
Proposed Patch

Attachment 344361 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8449999

New failing tests:
accessibility/mac/set-value-editable-dispatch-events.html

[EWS Watchlist]

Created attachment 344378 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 344361 [details]
Proposed Patch

Attachment 344361 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8451472

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html
http/tests/security/local-video-source-from-remote.html

[EWS Watchlist]

Created attachment 344394 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Alexey Proskuryakov]

Comment on attachment 344361 [details]
Proposed Patch

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

> Tools/ChangeLog:3
> +        [ews-build] EWS should unapply the patch and build ToT when patch fails to build

As discussed in person, I'm not sure what the full context for this behavior. Seems fine to land, but let's discuss the overall design soon.

> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:462
> +            ExpectShell(workdir='wkdir',
> +                        command=["perl", "Tools/Scripts/build-webkit", '--Release'],

This patch has a mix of single and double quotes in multiple places, could you please clean that up?

Also, isn't the --release all lower case?

> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:476
> +                        command=["perl", "Tools/Scripts/build-webkit", '--Debug'],

Ditto with regards to case.

[Aakash Jain]

Comment on attachment 344361 [details]
Proposed Patch

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

>> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:462
>> +                        command=["perl", "Tools/Scripts/build-webkit", '--Release'],
> 
> This patch has a mix of single and double quotes in multiple places, could you please clean that up?
Fixed
> 
> Also, isn't the --release all lower case?
Good catch. Fixed in every place in this file.

>> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:476
>> +                        command=["perl", "Tools/Scripts/build-webkit", '--Debug'],
> Ditto with regards to case.
Fixed.

[Aakash Jain]

Committed r233734: <https://trac.webkit.org/changeset/233734>

[Radar WebKit Bug Importer]

<rdar://problem/42080620>