Bug 187358 - [ews-build] EWS should unapply the patch and build ToT when patch fails to build
Summary: [ews-build] EWS should unapply the patch and build ToT when patch fails to build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-07-05 12:43 PDT by Aakash Jain
Modified: 2018-07-11 11:14 PDT (History)
6 users (show)

See Also:


Attachments
Proposed Patch (4.95 KB, patch)
2018-07-05 13:40 PDT, Aakash Jain
ap: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews113 for mac-sierra (3.01 MB, application/zip)
2018-07-05 16:28 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews206 for win-future (12.92 MB, application/zip)
2018-07-05 19:27 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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>