Created attachment 368790 [details] Screenshot - Not submitted to New EWS Ran `Tools/Scripts/webkit-patch upload -g HEAD --no-review 197480` today, here's the output, emphasis mine: $ Tools/Scripts/webkit-patch upload -g HEAD --no-review 197480 Total errors found: 0 in 23 files 4 files to edit Was that diff correct? [Y/n]: Y Reading Keychain for bugs.webkit.org account and password. Click "Allow" to continue... Could not find a keychain entry for bugs.webkit.org. Reading Keychain for bugs.webkit.org account and password. Click "Allow" to continue... Logging in as dbates@webkit.org... Fetching: https://bugs.webkit.org/show_bug.cgi?id=197480&ctype=xml&excludefield=attachmentdata Obsoleting 1 old patch on bug 197480 Obsoleting attachment: 368720 Fetching: https://bugs.webkit.org/show_bug.cgi?id=197480&ctype=xml&excludefield=attachmentdata Adding patch "Patch" to https://bugs.webkit.org/show_bug.cgi?id=197480 Fetching: https://bugs.webkit.org/attachment.cgi?id=368787&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=197480&ctype=xml&excludefield=attachmentdata ***Submitting attachment 368787 [details] to EWS queues*** The patch was submitted to the old EWS, but was not submitted to the new EWS.
<rdar://problem/50424887>
Created attachment 368844 [details] Patch
(In reply to Aakash Jain from comment #2) > Created attachment 368844 [details] This patch was uploaded using the 'webkit-patch upload --no-review' command. It did submit the patch to both old and new EWS. Logs: [~/code/OpenSource/Tools]$webkit-patch upload --no-review 197519 Total errors found: 0 in 11 files Was that diff correct? [Y/n]: Reading Keychain for bugs.webkit.org account and password. Click "Allow" to continue... Could not find a keychain entry for bugs.webkit.org. Reading Keychain for bugs.webkit.org account and password. Click "Allow" to continue... Logging in as aakash_jain@apple.com... Fetching: https://bugs.webkit.org/show_bug.cgi?id=197519&ctype=xml&excludefield=attachmentdata Fetching: https://bugs.webkit.org/show_bug.cgi?id=197519&ctype=xml&excludefield=attachmentdata Adding patch "Patch" to https://bugs.webkit.org/show_bug.cgi?id=197519 Fetching: https://bugs.webkit.org/attachment.cgi?id=368844&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=197519&ctype=xml&excludefield=attachmentdata Submitting attachment 368844 [details] to EWS queues Submitting attachment 368844 [details] to EWS queues
Comment on attachment 368844 [details] Patch Attachment 368844 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12070395 New failing tests: http/tests/css/filters-on-iframes.html
Created attachment 368860 [details] Archive of layout-test-results from ews212 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews212 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment on attachment 368844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368844&action=review > Tools/ChangeLog:17 > + * Scripts/webkitpy/common/net/ewsserver_unittest.py: Added unit-test. Unit test for the EWSServer class or unit test for the MockEWSServer class? > Tools/Scripts/webkitpy/common/net/ewsserver.py:35 > + self.use_https = bool(use_https) Would it ever be valid to NOT use https here? > Tools/Scripts/webkitpy/common/net/ewsserver_mock.py:32 > + self.use_https = True Why do we need use_https? > Tools/Scripts/webkitpy/common/net/ewsserver_unittest.py:34 > + self.assertTrue(ews_server.use_https) This doesn't seem like a very useful test. I think we could actually try the submit_to_ews(...) function, I think that MockBrowser() provides enough to let that work.
Comment on attachment 368844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368844&action=review >> Tools/ChangeLog:17 >> + * Scripts/webkitpy/common/net/ewsserver_unittest.py: Added unit-test. > > Unit test for the EWSServer class or unit test for the MockEWSServer class? For EWSServer class. >> Tools/Scripts/webkitpy/common/net/ewsserver.py:35 >> + self.use_https = bool(use_https) > > Would it ever be valid to NOT use https here? It might be valid if for some reason https is not configured properly on the server (e.g.: submitting the patches to a UAT instance of EWS). I can't think of another strong reason. I was trying to keep it consistent with similar statusserver code. use_https was added by Dan Bates in r232979. I believe he might have encountered a scenario in which there was a need to use http instead of https. >> Tools/Scripts/webkitpy/common/net/ewsserver_mock.py:32 >> + self.use_https = True > > Why do we need use_https? Removing in updated patch. >> Tools/Scripts/webkitpy/common/net/ewsserver_unittest.py:34 >> + self.assertTrue(ews_server.use_https) > > This doesn't seem like a very useful test. I think we could actually try the submit_to_ews(...) function, I think that MockBrowser() provides enough to let that work. Improved unit-test little bit, it now executes the submit_to_ews method. Note that upload_unittest.py also cover submit_to_ews to some extent.
Created attachment 368985 [details] Patch
(In reply to Aakash Jain from comment #7) ... > > > > Would it ever be valid to NOT use https here? > > It might be valid if for some reason https is not configured properly on the > server (e.g.: submitting the patches to a UAT instance of EWS). I can't > think of another strong reason. I was trying to keep it consistent with > similar statusserver code. use_https was added by Dan Bates in r232979. I > believe he might have encountered a scenario in which there was a need to > use http instead of https. I hadn't thought about UAT, that would be a good reason.
Looks good to me, unofficial r+.
Comment on attachment 368985 [details] Patch Clearing flags on attachment: 368985 Committed r244923: <https://trac.webkit.org/changeset/244923>
All reviewed patches have been landed. Closing bug.