Skip to content

Commit

Permalink
Fix wptserve in run_webdriver_tests
Browse files Browse the repository at this point in the history
Following https://crrev.com/c/1641436, wptserve needs to have an output
directory to redirect its logs to (same as Apache). However,
run_webdriver_tests.py didn't set the output directory of the port
object ("results-directory") previously, so it would default to
out/Release/layout-test-results, which does not exist on the bot.

This change adds a new --output-dir argument to run_webdriver_tests.py
and sets it to ISOLATED_OUTDIR on Swarming and a temporary directory
locally (when the argument isn't present).

In addition, wptserve.py is reverted back to conditionally add
--ws_doc_root as run_webdriver_tests does not have WebSocket handlers.

Drive-by change: remove the redundant --isolated-script-test-output
flag from gn_isolate_map.pyl as the isolated script runner already
adds that flag automatically.

Bug: 968904, 972005
Change-Id: Id066981b7eee6585de56dfa62e750b367b965843
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1649382
Reviewed-by: John Chen <johnchen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Auto-Submit: Robert Ma <robertma@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#668016}
  • Loading branch information
Hexcles authored and Commit Bot committed Jun 11, 2019
1 parent 502e6e4 commit 194bd25
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 5 deletions.
10 changes: 10 additions & 0 deletions chrome/test/chromedriver/test/run_webdriver_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import argparse
import sys
import json
import tempfile
import time
import logging

Expand Down Expand Up @@ -181,6 +182,9 @@ def run_test(path, path_finder, port, skipped_tests=[]):
help='Output verbose server logs to this file')
parser.add_argument(
'--chrome', help='Path to chrome binary')
parser.add_argument(
'--output-dir',
help='Output directory for misc logs (e.g. wptserve)')
parser.add_argument(
'--isolated-script-test-output',
help='JSON output file used by swarming')
Expand Down Expand Up @@ -211,6 +215,12 @@ def run_test(path, path_finder, port, skipped_tests=[]):

host = Host()
port = host.port_factory.get()
if options.output_dir:
port.set_option_default('results_directory', options.output_dir)
else:
output_dir = tempfile.mkdtemp('webdriver_tests')
_log.info('Using a temporary output dir %s', output_dir)
port.set_option_default('results_directory', output_dir)
path_finder = PathFinder(host.filesystem)

# Starts WPT Serve to serve the WPT WebDriver test content.
Expand Down
2 changes: 1 addition & 1 deletion testing/buildbot/gn_isolate_map.pyl
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@
"../../chrome/test/chromedriver/test/run_webdriver_tests.py",
"-v",
"--chromedriver=chromedriver",
"--isolated-script-test-output=${ISOLATED_OUTDIR}/results.json",
"--output-dir=${ISOLATED_OUTDIR}",
"--test-path=../../third_party/blink/web_tests/external/wpt/webdriver/tests/",
],
},
Expand Down
8 changes: 6 additions & 2 deletions third_party/blink/tools/blinkpy/web_tests/servers/wptserve.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,12 @@ def __init__(self, port_obj, output_dir):
start_cmd = [self._port_obj.host.executable,
'-u', wpt_script, 'serve',
'--config', self._config_file,
'--doc_root', path_to_wpt_tests,
'--ws_doc_root', path_to_ws_handlers]
'--doc_root', path_to_wpt_tests]

# Some users (e.g. run_webdriver_tests.py) do not need WebSocket
# handlers, so we only add the flag if the directory exists.
if self._port_obj.host.filesystem.exists(path_to_ws_handlers):
start_cmd += ['--ws_doc_root', path_to_ws_handlers]

# TODO(burnik): We should stop setting the CWD once WPT can be run without it.
self._cwd = path_to_wpt_root
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,24 @@ def setUp(self):

# pylint: disable=protected-access

def test_init_start_cmd(self):
def test_init_start_cmd_without_ws_handlers(self):
server = WPTServe(self.port, '/foo')
self.assertEqual(
server._start_cmd, # pylint: disable=protected-access
[
'python',
'-u',
'/mock-checkout/third_party/blink/tools/blinkpy/third_party/wpt/wpt/wpt',
'serve',
'--config',
server._config_file,
'--doc_root',
'/test.checkout/wtests/external/wpt',
])

def test_init_start_cmd_with_ws_handlers(self):
self.host.filesystem.maybe_make_directory(
'/test.checkout/wtests/external/wpt/websockets/handlers')
server = WPTServe(self.port, '/foo')
self.assertEqual(
server._start_cmd, # pylint: disable=protected-access
Expand All @@ -39,7 +56,7 @@ def test_init_start_cmd(self):
'--doc_root',
'/test.checkout/wtests/external/wpt',
'--ws_doc_root',
'/test.checkout/wtests/external/wpt/websockets/handlers'
'/test.checkout/wtests/external/wpt/websockets/handlers',
])

def test_init_gen_config(self):
Expand Down

0 comments on commit 194bd25

Please sign in to comment.