Bug 187358

Summary: [ews-build] EWS should unapply the patch and build ToT when patch fails to build
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, ddkilzer, ews-watchlist, lforschler, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=187357
https://bugs.webkit.org/show_bug.cgi?id=187549
Attachments:
Description Flags
Proposed Patch
ap: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews113 for mac-sierra
none
Archive of layout-test-results from ews206 for win-future none

Description Aakash Jain 2018-07-05 12:43:51 PDT
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.
Comment 1 Aakash Jain 2018-07-05 13:40:03 PDT
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
Comment 2 EWS Watchlist 2018-07-05 13:42:48 PDT
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.
Comment 3 Aakash Jain 2018-07-05 13:45:08 PDT
Style queue messages are false positive.

UnApplyPatchIfRequired variable is defined in https://bugs.webkit.org/show_bug.cgi?id=187357
Comment 4 EWS Watchlist 2018-07-05 16:28:01 PDT Comment hidden (obsolete)
Comment 5 EWS Watchlist 2018-07-05 16:28:02 PDT Comment hidden (obsolete)
Comment 6 EWS Watchlist 2018-07-05 19:26:58 PDT
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
Comment 7 EWS Watchlist 2018-07-05 19:27:10 PDT
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
Comment 8 Alexey Proskuryakov 2018-07-10 16:39:06 PDT
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.
Comment 9 Aakash Jain 2018-07-11 11:11:37 PDT
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.
Comment 10 Aakash Jain 2018-07-11 11:12:29 PDT
Committed r233734: <https://trac.webkit.org/changeset/233734>
Comment 11 Radar WebKit Bug Importer 2018-07-11 11:14:33 PDT
<rdar://problem/42080620>