Bug 208693 - [webkitpy] Fix executive on Windows to run wpt server correctly
Summary: [webkitpy] Fix executive on Windows to run 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
Depends on:
Blocks:
 
Reported: 2020-03-05 21:05 PST by Basuke Suzuki
Modified: 2020-03-06 17:54 PST (History)
10 users (show)

See Also:


Attachments
PATCH (2.23 KB, patch)
2020-03-06 16:10 PST, Basuke Suzuki
no flags Details | Formatted Diff | Diff
PATCH (3.69 KB, patch)
2020-03-06 16:21 PST, Basuke Suzuki
no flags Details | Formatted Diff | Diff
PATCH (3.69 KB, patch)
2020-03-06 16:53 PST, 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 2020-03-05 21:05:06 PST
Caused by typo.
Comment 1 Basuke Suzuki 2020-03-06 16:10:44 PST
Created attachment 392796 [details]
PATCH
Comment 2 Fujii Hironori 2020-03-06 16:18:25 PST
Comment on attachment 392796 [details]
PATCH

View in context: https://bugs.webkit.org/attachment.cgi?id=392796&action=review

> Source/WebCore/loader/cache/CachedImage.cpp:569
> +    }

Whats this?
Comment 3 Basuke Suzuki 2020-03-06 16:21:04 PST
Created attachment 392798 [details]
PATCH
Comment 4 Basuke Suzuki 2020-03-06 16:23:32 PST
Sorry, I submitted a wrong file X( New one is good to go.
Comment 5 Jonathan Bedard 2020-03-06 16:33:15 PST
Comment on attachment 392798 [details]
PATCH

View in context: https://bugs.webkit.org/attachment.cgi?id=392798&action=review

> Tools/Scripts/webkitpy/common/system/executive.py:324
> +    def _windows_killCommand(self):

If this is an instance function, we shouldn't need the 'self._is_native_win' in kill_all.

Otherwise it should be a staticmethod or a class variable (Like what we did with PIPE and STDOUT)
Comment 6 Basuke Suzuki 2020-03-06 16:53:28 PST
Created attachment 392812 [details]
PATCH
Comment 7 Jonathan Bedard 2020-03-06 16:56:25 PST
Comment on attachment 392812 [details]
PATCH

View in context: https://bugs.webkit.org/attachment.cgi?id=392812&action=review

> Tools/Scripts/webkitpy/common/system/executive.py:326
> +            return os.path.join("C:", os.sep, "WINDOWS", "system32", "taskkill.exe")

Do we actually need os.sep here? I would expect os.path.join(...) to do that for you.
Comment 8 Basuke Suzuki 2020-03-06 17:02:07 PST
Comment on attachment 392812 [details]
PATCH

View in context: https://bugs.webkit.org/attachment.cgi?id=392812&action=review

>> Tools/Scripts/webkitpy/common/system/executive.py:326
>> +            return os.path.join("C:", os.sep, "WINDOWS", "system32", "taskkill.exe")
> 
> Do we actually need os.sep here? I would expect os.path.join(...) to do that for you.

Well that part is copied from existing code, which is working. So I left untouched that.
Comment 9 WebKit Commit Bot 2020-03-06 17:53:56 PST
Comment on attachment 392812 [details]
PATCH

Clearing flags on attachment: 392812

Committed r258045: <https://trac.webkit.org/changeset/258045>
Comment 10 WebKit Commit Bot 2020-03-06 17:53:58 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2020-03-06 17:54:17 PST
<rdar://problem/60178285>