Bug 187973 - [webkitpy][Win] LayoutTests: test names should be Unix style, separated by slash not backslash
Summary: [webkitpy][Win] LayoutTests: test names should be Unix style, separated by sl...
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: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks: 188345
  Show dependency treegraph
 
Reported: 2018-07-24 15:02 PDT by Basuke Suzuki
Modified: 2018-08-14 15:45 PDT (History)
10 users (show)

See Also:


Attachments
WIP patch (3.56 KB, patch)
2018-08-07 02:47 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (4.51 KB, patch)
2018-08-07 03:12 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (3.92 KB, patch)
2018-08-08 01:26 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (6.49 KB, patch)
2018-08-08 21:02 PDT, Fujii Hironori
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-07-24 15:02:20 PDT
Test path on native Windows are displayed using back slash. It should be forward slash like other ports do.
Comment 1 Fujii Hironori 2018-08-06 23:49:51 PDT
relative_test_filename of chromium_win.py used to replace backward slash with forward slash. (Bug 34739)
Comment 2 Fujii Hironori 2018-08-07 00:47:08 PDT
test_name has been cannibalized in unix-style since Bug 63597.

Basuke's Bug 179572 seems bad fix.
Comment 3 Fujii Hironori 2018-08-07 00:57:48 PDT
Bug 180660, too.
Comment 4 Fujii Hironori 2018-08-07 02:47:28 PDT
Created attachment 346701 [details]
WIP patch
Comment 5 Fujii Hironori 2018-08-07 03:12:14 PDT
Created attachment 346702 [details]
WIP patch

* Reverted parts of Bug 179572, Bug 180660 and Bug 181814 to restore test_name is unix-style.
* relative_test_filename: Replace os.path.sep with self.TEST_PATH_SEPARATOR.
* abspath_for_test: Replace self.TEST_PATH_SEPARATOR with os.path.sep.
Comment 6 Fujii Hironori 2018-08-08 01:06:50 PDT
This patch makes some tests of test-webkitpy failing because 
common/system/filesystem_mock.py assumes path separator is a forward slash.

> python Tools\Scripts\test-webkitpy webkitpy.port.base_unittest.PortTest.test_test_exists

This is the test case to check mock file system.

This seems the reason why chromium_win.py didn't do the string conversion in abspath_for_test.
Comment 7 Fujii Hironori 2018-08-08 01:26:42 PDT
Created attachment 346760 [details]
WIP patch

This patch significantly reduces the number of test-webkitpy failures, but LayoutTest doesn't work.

FAILED (failures=41, errors=25)
↓
FAILED (failures=0, errors=8)
Comment 8 Fujii Hironori 2018-08-08 02:24:06 PDT
If DumpRenderTree is given a file path containing forward slashes, such like:

> C:\webkit\ga\LayoutTests\fast/css/button-height.html

CFURLCreateWithFileSystemPath converts it to "file://localhost/C:/webkit/ga/LayoutTests/fast%2Fcss%2Fbutton-height.html".

https://github.com/WebKit/webkit/blob/82bae82cf0f329dbe21059ef0986c4e92fea4ba6/Tools/DumpRenderTree/win/DumpRenderTree.cpp#L1135
> url = CFURLCreateWithFileSystemPath(0, str, kCFURLWindowsPathStyle, false);
Comment 9 Fujii Hironori 2018-08-08 21:02:27 PDT
Created attachment 346817 [details]
Patch
Comment 10 Fujii Hironori 2018-08-09 19:14:33 PDT
Comment on attachment 346817 [details]
Patch

Clearing flags on attachment: 346817

Committed r234749: <https://trac.webkit.org/changeset/234749>
Comment 11 Fujii Hironori 2018-08-09 19:14:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2018-08-09 19:15:20 PDT
<rdar://problem/43122448>
Comment 13 Per Arne Vollan 2018-08-10 08:49:09 PDT
I am currently observing errors on the Win EWS bots. Is it possible that this was introduced by this change? This is the error message:

Starting WebSocket server ...
Failed to remove stale pywebsocket log files: [Errno 2] No such file or directory: '/cygdrive/C/cygwin/home/buildbot/WebKit/WebKitBuild/Release/bin32/layout-test-results'

IOError raised: [Errno 2] No such file or directory: u'/cygdrive/C/cygwin/home/buildbot/WebKit/WebKitBuild/Release/bin32/layout-test-results/pywebsocket.ws.log-out.txt'
Traceback (most recent call last):
  File "/home/buildbot/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 85, in main
    run_details = run(port, options, args, stderr)
  File "/home/buildbot/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 447, in run
    run_details = manager.run(args)
  File "/home/buildbot/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 240, in run
    needs_http=needs_http, needs_web_platform_test_server=needs_web_platform_test_server, needs_websockets=needs_websockets)
  File "/home/buildbot/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 87, in __init__
    self.start_servers()
  File "/home/buildbot/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 202, in start_servers
    self._port.start_websocket_server()
  File "/home/buildbot/WebKit/Tools/Scripts/webkitpy/port/base.py", line 1031, in start_websocket_server
    server.start()
  File "/home/buildbot/WebKit/Tools/Scripts/webkitpy/layout_tests/servers/http_server_base.py", line 95, in start
    self._prepare_config()
  File "/home/buildbot/WebKit/Tools/Scripts/webkitpy/layout_tests/servers/websocket_server.py", line 118, in _prepare_config
    self._wsout = self._filesystem.open_text_file_for_writing(output_log)
  File "/home/buildbot/WebKit/Tools/Scripts/webkitpy/common/system/filesystem.py", line 239, in open_text_file_for_writing
    return codecs.open(path, 'w', 'utf8')
  File "/usr/lib/python2.7/codecs.py", line 896, in open
    file = __builtin__.open(filename, mode, buffering)
IOError: [Errno 2] No such file or directory: u'/cygdrive/C/cygwin/home/buildbot/WebKit/WebKitBuild/Release/bin32/layout-test-results/pywebsocket.ws.log-out.txt'
Comment 15 Truitt Savell 2018-08-10 11:56:44 PDT
Reverted r234749 for reason:

Caused all perf tests to fail in Sierra

Committed r234765: <https://trac.webkit.org/changeset/234765>
Comment 16 Basuke Suzuki 2018-08-14 11:38:40 PDT
This patch alsodoesn't solve my original issue. The test result is still displayed using Windows path. Something is missing.

> Running 8 DumpRenderTrees in parallel.
> 
> [63/270] http\tests\xmlhttprequest\cache-override.html passed unexpectedly
> [143/270] http\tests\xmlhttprequest\range-test.html passed unexpectedly
> [169/270] http\tests\xmlhttprequest\response-empty-arraybuffer.html passed unexpectedly
> [258/270] http\tests\xmlhttprequest\reentrant-cancel.html failed unexpectedly (test timed out, test was not run)
> [270/270] http\tests\xmlhttprequest\xmlhttprequest-overridemimetype-content-type-header.html failed unexpectedly (test timed out, test was not run)
> [270/270] http\tests\xmlhttprequest\on-network-timeout-error-during-preflight.html failed unexpectedly (test timed out, text diff)
> 
> Retrying 3 unexpected failures ...
> 
> Running 1 DumpRenderTree.
> 
> [3/3] http\tests\xmlhttprequest\on-network-timeout-error-during-preflight.html failed unexpectedly (test timed out, text diff)
> 
> 264 tests ran as expected, 6 didn't:
> 
> Expected to fail, but passed: (3)
>   http\tests\xmlhttprequest\cache-override.html
>   http\tests\xmlhttprequest\range-test.html
>   http\tests\xmlhttprequest\response-empty-arraybuffer.html
> 
> Unexpected flakiness: timeouts (2)
>   http\tests\xmlhttprequest\reentrant-cancel.html [ Timeout Pass ]
>   http\tests\xmlhttprequest\xmlhttprequest-overridemimetype-content-type-header.html [ Timeout Pass ]
> 
> Regressions: Unexpected timeouts (1)
>   http\tests\xmlhttprequest\on-network-timeout-error-during-preflight.html [ Timeout ]
Comment 17 Basuke Suzuki 2018-08-14 11:39:20 PDT
Oh, it was rolled back. That's why.
Comment 18 Ross Kirsling 2018-08-14 15:01:35 PDT
Comment on attachment 346817 [details]
Patch

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

> Tools/Scripts/webkitpy/port/base.py:844
> +        return host.filesystem.join(host.filesystem.map_base_host_path(self.layout_tests_dir()), *test_name.split(self.TEST_PATH_SEPARATOR))

Looks like perf tests use abspaths as test names, so we need to be explicit and `.replace(self.TEST_PATH_SEPARATOR, self.host.filesystem.sep)` here.
Comment 19 Ross Kirsling 2018-08-14 15:45:15 PDT
Committed r234863: <https://trac.webkit.org/changeset/234863>