Bug 190248 - [LayoutTests][Win] Stop wpt server correctly.
Summary: [LayoutTests][Win] Stop wpt server correctly.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basuke Suzuki
URL:
Keywords: InRadar
: 190236 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-10-03 07:21 PDT by Basuke Suzuki
Modified: 2018-10-04 09:57 PDT (History)
12 users (show)

See Also:


Attachments
PATCH (7.96 KB, patch)
2018-10-03 07:28 PDT, Basuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Basuke Suzuki 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().
Comment 1 Basuke Suzuki 2018-10-03 07:28:58 PDT
Created attachment 351518 [details]
PATCH
Comment 2 EWS Watchlist 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.
Comment 3 Basuke Suzuki 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
Comment 4 Don Olmstead 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.
Comment 5 Basuke Suzuki 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.
Comment 6 Fujii Hironori 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.
Comment 7 Fujii Hironori 2018-10-03 19:46:26 PDT
*** Bug 190236 has been marked as a duplicate of this bug. ***
Comment 8 WebKit Commit Bot 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>
Comment 9 WebKit Commit Bot 2018-10-04 09:56:25 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-10-04 09:57:25 PDT
<rdar://problem/45011998>