Bug 143516 - [Win] Allow running tests without Cygwin
Summary: [Win] Allow running tests without Cygwin
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-07 23:38 PDT by Myles C. Maxfield
Modified: 2016-02-14 13:34 PST (History)
3 users (show)

See Also:

Attachments
Patch (5.81 KB, patch)
2015-04-07 23:43 PDT, Myles C. Maxfield 		bfulgham: review-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2015-04-07 23:38:15 PDT 
[Win] Allow running tests without Cygwin
Comment 1 Myles C. Maxfield 2015-04-07 23:43:47 PDT 
Created attachment 250338 [details]
Patch
Comment 2 WebKit Commit Bot 2015-04-07 23:45:16 PDT 
Attachment 250338 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/port/win.py:259:  [WinPort.setup_test_run] Undefined variable 'WindowsError'  [pylint/E0602] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Brent Fulgham 2015-04-08 12:48:10 PDT 
Comment on attachment 250338 [details]
Patch

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

This looks very good, but I think it will break Cygwin support. We need to check for running Windows (as opposed to Cygwin) and use the proper PID in each case.

If you grep through the webkitpy stuff, you should find some examples of 'Cygwin'-specific (as opposed to Windows-specific) routines.

> Tools/Scripts/webkitpy/port/win.py:94
> +        env = os.environ

I'm not sure about this. We are supposed to be setting things like WEBKIT_TEST_FONTS and so forth. Are these getting set properly?

> Tools/Scripts/webkitpy/port/win.py:-330
> -        return system_pid

We can't remove this until we are off Cygwin entirely. I think we have a distinction between Cygwin and Windows. This code should be executed for Cygwin, but not for oure Windows.
Comment 4 Myles C. Maxfield 2016-02-14 13:34:31 PST 
Looks like this has since been fixed.