Bug 85333 – nrwt: make ServerProcess work on chromium win

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dirk Pranke]

nrwt: make ServerProcess work on chromium win

[Dirk Pranke]

Created attachment 139722 [details]
Patch

[Ojan Vafai]

Comment on attachment 139722 [details]
Patch

I suppose there's no way to write basic tests for this? It'd be fine for the tests to check sys.platform first to isolate them to only run on windows. Even something simple that just calls the code and makes sure it doesn't error is better than nothing IMO.

[Tony Chang]

Comment on attachment 139722 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/port/server_process.py:45
> +    import win32event

Nit: win32event doesn't appear to be used.

> Tools/Scripts/webkitpy/layout_tests/port/server_process.py:244
> +            if e[0] not in (109, errno.ESHUTDOWN):

What's 109?

> Tools/Scripts/webkitpy/layout_tests/port/server_process.py:273
> +                self._wait_for_data_and_update_buffers(deadline)

Nit: Maybe rename this method _posix?

[Dirk Pranke]

(In reply to comment #2)
> (From update of attachment 139722 [details])
> I suppose there's no way to write basic tests for this? It'd be fine for the tests to check sys.platform first to isolate them to only run on windows. Even something simple that just calls the code and makes sure it doesn't error is better than nothing IMO.

I will add something.

(In reply to comment #3)
> (From update of attachment 139722 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=139722&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/port/server_process.py:45
> > +    import win32event
> 
> Nit: win32event doesn't appear to be used.
> 

Good catch, that's left over from a previous approach. I will remove it.

> > Tools/Scripts/webkitpy/layout_tests/port/server_process.py:244
> > +            if e[0] not in (109, errno.ESHUTDOWN):
> 
> What's 109?
>

Dunno :) This was cribbed from some ActiveState sample code that I apparently dropped the comments for. I will add those back in and attempt to track down what this code refers to.
 
> > Tools/Scripts/webkitpy/layout_tests/port/server_process.py:273
> > +                self._wait_for_data_and_update_buffers(deadline)
> 
> Nit: Maybe rename this method _posix?

I don't really like "posix" since windows is theoretically posix-compliant. I will attempt to rename it to something else more meaningful, though.

[Tony Chang]

It looks like there are unit tests that are scattered around that can be enabled.  E.g., run_webkit_tests_integrationtest.py.

[Dirk Pranke]

Created attachment 139900 [details]
patch for landing

[Dirk Pranke]

Committed r115903: <http://trac.webkit.org/changeset/115903>