Bug 190248

Summary: [LayoutTests][Win] Stop wpt server correctly.
Product: WebKit Reporter: Basuke Suzuki <basuke>
Component: Tools / TestsAssignee: Basuke Suzuki <basuke>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, basuke, bfulgham, commit-queue, ddkilzer, don.olmstead, ews-watchlist, fujii.hironori, glenn, lforschler, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
PATCH none

Basuke Suzuki
Reported 2018-10-03 07:21:55 PDT
The wpt test server wasn't killed correctly which keep opening the log files and prevent following `archive-test-results` command. It is caused by mis-implementation of executive.interrupt().
Attachments
PATCH (7.96 KB, patch)
2018-10-03 07:28 PDT, Basuke Suzuki 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Basuke Suzuki
Comment 1 2018-10-03 07:28:58 PDT
Created attachment 351518 [details] PATCH
EWS Watchlist
Comment 2 2018-10-03 07:31:28 PDT
Attachment 351518 [details] did not pass style-queue: ERROR: Tools/Scripts/webkitpy/common/system/executive.py:312: [Executive.interrupt] Module 'signal' has no 'CTRL_C_EVENT' member [pylint/E1101] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Basuke Suzuki
Comment 3 2018-10-03 07:35:38 PDT
(In reply to Build Bot from comment #2) > Attachment 351518 [details] did not pass style-queue: > > > ERROR: Tools/Scripts/webkitpy/common/system/executive.py:312: > [Executive.interrupt] Module 'signal' has no 'CTRL_C_EVENT' member > [pylint/E1101] [5] > Total errors found: 1 in 2 files > > > If any of these errors are false positives, please file a bug against > check-webkit-style. This is false positive. The constant is available only on Windows. https://docs.python.org/2/library/signal.html#signal.CTRL_C_EVENT
Don Olmstead
Comment 4 2018-10-03 08:28:50 PDT
Comment on attachment 351518 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=351518&action=review Informal review is good. Just wondering if there's any way to just check if its windows native or cygwin in one line. > Tools/Scripts/webkitpy/common/system/executive.py:487 > + if self._is_cygwin or self._is_native_win: Is there any query for both at the same time? This is repeated in a couple places.
Basuke Suzuki
Comment 5 2018-10-03 10:09:30 PDT
Comment on attachment 351518 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=351518&action=review >> Tools/Scripts/webkitpy/common/system/executive.py:487 >> + if self._is_cygwin or self._is_native_win: > > Is there any query for both at the same time? This is repeated in a couple places. In platformInfo, is_win() is also defined so that it should be. But after writing the code, I didn't read this code hard. I am okay for both.
Fujii Hironori
Comment 6 2018-10-03 19:45:48 PDT
I also confirmed your patch solves the pywebsocket.ws.log-err.txt issue (Bug 190236). Thank you for working on this.
Fujii Hironori
Comment 7 2018-10-03 19:46:26 PDT
*** Bug 190236 has been marked as a duplicate of this bug. ***
WebKit Commit Bot
Comment 8 2018-10-04 09:56:23 PDT
Comment on attachment 351518 [details] PATCH Clearing flags on attachment: 351518 Committed r236835: <https://trac.webkit.org/changeset/236835>
WebKit Commit Bot
Comment 9 2018-10-04 09:56:25 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2018-10-04 09:57:25 PDT
<rdar://problem/45011998>
Note You need to log in before you can comment on or make changes to this bug.