Bug 187973

Summary: [webkitpy][Win] LayoutTests: test names should be Unix style, separated by slash not backslash
Product: WebKit Reporter: Basuke Suzuki <basuke>
Component: Tools / TestsAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, ews-watchlist, glenn, Hironori.Fujii, lforschler, pvollan, ross.kirsling, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 188345    
Attachments:
Description Flags
WIP patch
none
WIP patch
none
WIP patch
none
Patch none

Basuke Suzuki
Reported 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.
Attachments
WIP patch (3.56 KB, patch)
2018-08-07 02:47 PDT, Fujii Hironori
no flags
WIP patch (4.51 KB, patch)
2018-08-07 03:12 PDT, Fujii Hironori
no flags
WIP patch (3.92 KB, patch)
2018-08-08 01:26 PDT, Fujii Hironori
no flags
Patch (6.49 KB, patch)
2018-08-08 21:02 PDT, Fujii Hironori
no flags
Fujii Hironori
Comment 1 2018-08-06 23:49:51 PDT
relative_test_filename of chromium_win.py used to replace backward slash with forward slash. (Bug 34739)
Fujii Hironori
Comment 2 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.
Fujii Hironori
Comment 3 2018-08-07 00:57:48 PDT
Fujii Hironori
Comment 4 2018-08-07 02:47:28 PDT
Created attachment 346701 [details] WIP patch
Fujii Hironori
Comment 5 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.
Fujii Hironori
Comment 6 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.
Fujii Hironori
Comment 7 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)
Fujii Hironori
Comment 8 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);
Fujii Hironori
Comment 9 2018-08-08 21:02:27 PDT
Fujii Hironori
Comment 10 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>
Fujii Hironori
Comment 11 2018-08-09 19:14:36 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12 2018-08-09 19:15:20 PDT
Per Arne Vollan
Comment 13 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'
Truitt Savell
Comment 15 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>
Basuke Suzuki
Comment 16 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 ]
Basuke Suzuki
Comment 17 2018-08-14 11:39:20 PDT
Oh, it was rolled back. That's why.
Ross Kirsling
Comment 18 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.
Ross Kirsling
Comment 19 2018-08-14 15:45:15 PDT
Note You need to log in before you can comment on or make changes to this bug.