1479955 - MarionetteTestharnessExecutor sets a timeout of 0 when running with a debugger

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

MarionetteTestharnessExecutor has this code:

    timeout = (test.timeout * self.timeout_multiplier if self.debug_info is None
               else None)

Clearly the intent is to avoid setting a timeout if a debugger is being used. However, this None later gets converted to "null" when passed as %(timeout)s in testharness_webdriver.js. That script then passes null directly to window.setTimeout(), which treats it as 0. I.e. instead of suppressing the timeout, it fires immediately.

A patch like this is probably needed:

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/testharness_webdriver.js b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/testharness_webdriver.js
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/testharness_webdriver.js
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/testharness_webdriver.js
@@ -12,12 +12,14 @@ window.setMessageListener = function(fun
 };
 
 window.setMessageListener(function(event) {
   window.message_queue.push(event);
 });
 
 window.win = window.open("%(abs_url)s", "%(window_id)s");
 
-window.timer = setTimeout(function() {
-  window.win.timeout();
-  window.win.close();
-}, %(timeout)s);
+if (%(timeout)s != null) {
+  window.timer = setTimeout(function() {
+    window.win.timeout();
+    window.win.close();
+  }, %(timeout)s);
+}

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

James, could you have a look?

Comment 2

[James Graham [:jgraham]]

Oops. The patch looks right and worked when I tested it so I applied it with --author=[e-mail]. I hope that's OK.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b069f55fad2c
MarionetteTestharnessExecutor sets a timeout of 0 when running with a debugger, r=jgraham

Comment 4

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12257 for changes under testing/web-platform/tests

Comment 5

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 6

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/b069f55fad2c

Comment 7

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged