Bug 187581 – webkitpy.port.server_process_unittest.TestServerProcess.test_basic failed on Windows Python

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Fujii Hironori]

webkitpy.port.server_process_unittest.TestServerProcess.test_basic failed on Windows Python

While it passes on Cygwin Python, it fails in Windows Python:

> C:\webkit>python Tools\Scripts\test-webkitpy -v webkitpy.port.server_process_unittest
> Skipping lldb_webkit tests; could not find path to lldb.py ''.
> Skipping QueueStatusServer tests; the Google AppEngine Python SDK is not installed.
> Suppressing most webkitpy logging while running unit tests.
> [1/5] webkitpy.port.server_process_unittest.TestServerProcess.test_basic failed:
>   Traceback (most recent call last):
>     File "C:\webkit\Tools\Scripts\webkitpy\port\server_process_unittest.py", line 111, in test_basic
>       self.assertEqual(proc.poll(), 0)
>   AssertionError: None != 0
> 
> [2/5] webkitpy.port.server_process_unittest.TestServerProcess.test_broken_pipe passed
> [3/5] webkitpy.port.server_process_unittest.TestServerProcess.test_cleanup passed
> [4/5] webkitpy.port.server_process_unittest.TestServerProcess.test_process_crashing passed
> [5/5] webkitpy.port.server_process_unittest.TestServerProcess.test_process_crashing_no_data passed
> Ran 5 tests in 0.484s
> FAILED (failures=1, errors=0)
> 
> C:\webkit>

[Fujii Hironori]

This test has been modifed for Windows Python.

Bug 151721 – [Win] Run non-http tests without Cygwin
http://trac.webkit.org/changeset/192944

[Fujii Hironori]

If I remove the condition "sys.platform.startswith('win')", another failure happens.
This means stderr is not ouput.

> C:\webkit>python Tools\Scripts\test-webkitpy -v webkitpy.port.server_process_unittest
> Skipping lldb_webkit tests; could not find path to lldb.py ''.
> Skipping QueueStatusServer tests; the Google AppEngine Python SDK is not installed.
> Suppressing most webkitpy logging while running unit tests.
> [1/5] webkitpy.port.server_process_unittest.TestServerProcess.test_basic erred:
>   Traceback (most recent call last):
>     File "C:\webkit\Tools\Scripts\webkitpy\port\server_process_unittest.py", line 122, in test_basic
>       self.assertEqual(line.strip(), "stderr")
>   AttributeError: 'NoneType' object has no attribute 'strip'
> 
> [2/5] webkitpy.port.server_process_unittest.TestServerProcess.test_broken_pipe passed
> [3/5] webkitpy.port.server_process_unittest.TestServerProcess.test_cleanup passed
> [4/5] webkitpy.port.server_process_unittest.TestServerProcess.test_process_crashing passed
> [5/5] webkitpy.port.server_process_unittest.TestServerProcess.test_process_crashing_no_data passed
> Ran 5 tests in 1.484s
> FAILED (failures=0, errors=1)

[Fujii Hironori]

Created attachment 344831 [details]
Patch

[Fujii Hironori]

All tests will pass after this patch applied both on Windows Python and Cygwin Python.

> C:\webkit>python Tools\Scripts\test-webkitpy -v webkitpy.port.server_process_unittest
> Skipping lldb_webkit tests; could not find path to lldb.py ''.
> Skipping QueueStatusServer tests; the Google AppEngine Python SDK is not installed.
> Suppressing most webkitpy logging while running unit tests.
> [1/5] webkitpy.port.server_process_unittest.TestServerProcess.test_basic passed
> [2/5] webkitpy.port.server_process_unittest.TestServerProcess.test_broken_pipe passed
> [3/5] webkitpy.port.server_process_unittest.TestServerProcess.test_cleanup passed
> [4/5] webkitpy.port.server_process_unittest.TestServerProcess.test_process_crashing passed
> [5/5] webkitpy.port.server_process_unittest.TestServerProcess.test_process_crashing_no_data passed
> Ran 5 tests in 0.639s
> 
> OK
> 
> C:\webkit>

[Fujii Hironori]

WinCairo BuildBots is failing with the same error.

https://build.webkit.org/builders/WinCairo%2064-bit%20WKL%20Release%20%28Tests%29/builds/559/steps/webkitpy-test/logs/stdio

> [877/1368] webkitpy.port.server_process_unittest.TestServerProcess.test_basic failed:
>   Traceback (most recent call last):
>     File "C:\WebKit-BuildWorker\wincairo-wkl-release-tests\build\Tools\Scripts\webkitpy\port\server_process_unittest.py", line 111, in test_basic
>       self.assertEqual(proc.poll(), 0)
>   AssertionError: None != 0

[Basuke Suzuki]

Comment on attachment 344831 [details]
Patch

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

> Tools/ChangeLog:19
> +        For failure #2, stdout also needs to be flushed as well as stdin.

Do you mean this?:        For failure #2, stderr also needs to be flushed as well as stdout.

[Fujii Hironori]

Created attachment 344936 [details]
Patch

Thank you for the review, Basuke. Addressed the review feedback.

[EWS Watchlist]

Comment on attachment 344936 [details]
Patch

Attachment 344936 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8526238

New failing tests:
http/tests/security/canvas-remote-read-remote-video-localhost.html

[EWS Watchlist]

Created attachment 344958 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Fujii Hironori]

Could anyone review?

[Fujii Hironori]

Could anyone review?

[Daniel Bates]

Comment on attachment 344936 [details]
Patch

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

> Tools/ChangeLog:23
> +        condition for Windows. Add a new test to check proc.poll() retruns

Retruns => returns

> Tools/Scripts/webkitpy/port/server_process_unittest.py:102
> +        cmd = [sys.executable, '-c', 'import sys; print "stdout"; sys.stdout.flush(); print >>sys.stderr, "stderr"; sys.stderr.flush(); sys.stdin.readline();']

Can you please explain why the flush is needed on Windows? I would have expected stderr to be unbuffered.

[Fujii Hironori]

Comment on attachment 344936 [details]
Patch

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

Thank you for the review.

>> Tools/ChangeLog:23
>> +        condition for Windows. Add a new test to check proc.poll() retruns
> 
> Retruns => returns

Will fix.

>> Tools/Scripts/webkitpy/port/server_process_unittest.py:102
>> +        cmd = [sys.executable, '-c', 'import sys; print "stdout"; sys.stdout.flush(); print >>sys.stderr, "stderr"; sys.stderr.flush(); sys.stdin.readline();']
> 
> Can you please explain why the flush is needed on Windows? I would have expected stderr to be unbuffered.

There are similar questions that stdout is buffered on pipe under Python for Windows.

  Python C program subprocess hangs at "for line in iter" - Stack Overflow
  https://stackoverflow.com/questions/20503671/python-c-program-subprocess-hangs-at-for-line-in-iter
  
  fflush - How fo force subprocess to refresh stdout buffer? - Stack Overflow
  https://stackoverflow.com/questions/22527010/how-fo-force-subprocess-to-refresh-stdout-buffer
  
  buffer - windows console program stdout is buffered when using pipe redirection - Stack Overflow
  https://stackoverflow.com/questions/9037177/windows-console-program-stdout-is-buffered-when-using-pipe-redirection

I found a useful command switch for this issue. I will remake the patch by using this command switch.

https://docs.python.org/2/using/cmdline.html

> -u 
> Force stdin, stdout and stderr to be totally unbuffered.

[Fujii Hironori]

Created attachment 345317 [details]
Patch

[EWS Watchlist]

Comment on attachment 345317 [details]
Patch

Attachment 345317 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8583488

New failing tests:
http/tests/preload/onload_event.html

[EWS Watchlist]

Created attachment 345330 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Fujii Hironori]

Daniel, could you review again?

[Fujii Hironori]

r?

[Daniel Bates]

Comment on attachment 345317 [details]
Patch

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

r=me

> Tools/Scripts/webkitpy/port/server_process_unittest.py:102
> +        cmd = [sys.executable, '-uc', 'import sys; print "stdout"; print >>sys.stderr, "stderr"; sys.stdin.readline();']

For your consideration, I suggest adding a comment to explain -u and why we are using -u.

> Tools/Scripts/webkitpy/port/server_process_unittest.py:149
> +        cmd = [sys.executable, '-uc', 'import sys; print "stdout 1"; print "stdout 2"; print "stdout 3"; sys.stdin.readline(); sys.exit(1);']

Ditto.

[Fujii Hironori]

Created attachment 345562 [details]
Patch to land

Thank you for the review and r+

[Fujii Hironori]

Comment on attachment 345562 [details]
Patch to land

Clearing flags on attachment: 345562

Committed r234130: <https://trac.webkit.org/changeset/234130>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/42525768>