Skip to content

Commit 2fd2aa1

Browse files
committed
Restore wpt window after each testharness test
This is the most straightforward possible approach. The problem is that a test might e.g. minimize the window and if it doesn't correctly cleanup after itself all subsequent tests run in an invalid state. Depending on the OS and WM this could even persist between browser restarts.
1 parent 85b2d4c commit 2fd2aa1

File tree

7 files changed

+59
-2
lines changed

7 files changed

+59
-2
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
[minimize-1.html]
2+
[Minimize a window]
3+
expected:
4+
if os == "android": FAIL

Diff for: infrastructure/window/minimize-1.html

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<!DOCTYPE html>
2+
<meta charset="utf-8">
3+
<title>Use testdriver to minimize the window</title>
4+
<script src="/resources/testharness.js"></script>
5+
<script src="/resources/testharnessreport.js"></script>
6+
<script src="/resources/testdriver.js"></script>
7+
<script src="/resources/testdriver-vendor.js"></script>
8+
<!-- This is really setup for the actual test which is test2.html which checks the harness restores the window -->
9+
<script>
10+
promise_test(async () => {
11+
await test_driver.minimize_window()
12+
assert_true(document.hidden);
13+
// And no cleanup
14+
}, "Minimize a window");
15+
</script>

Diff for: infrastructure/window/minimize-2.html

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<!DOCTYPE html>
2+
<meta charset="utf-8">
3+
<title>Use testdriver to check window is not minimized</title>
4+
<script src="/resources/testharness.js"></script>
5+
<script src="/resources/testharnessreport.js"></script>
6+
<script src="/resources/testdriver.js"></script>
7+
<script src="/resources/testdriver-vendor.js"></script>
8+
<!-- This only checks something meaningful when run after test1.html -->
9+
<script>
10+
promise_test(async () => {
11+
assert_false(document.hidden);
12+
}), "Test window is not minimized";
13+
</script>

Diff for: tools/wptrunner/wptrunner/browsers/firefox.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,8 @@ def executor_browser(self):
907907
extensions.append(self.specialpowers_path)
908908
return ExecutorBrowser, {"marionette_port": self.instance.marionette_port,
909909
"extensions": extensions,
910-
"supports_devtools": True}
910+
"supports_devtools": True,
911+
"supports_window_resize": True}
911912

912913
def check_crash(self, process, test):
913914
return log_gecko_crashes(self.logger,

Diff for: tools/wptrunner/wptrunner/browsers/firefox_android.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,8 @@ def executor_browser(self):
356356
# We never want marionette to install extensions because
357357
# that doesn't work on Android; instead they are in the profile
358358
"extensions": [],
359-
"supports_devtools": False}
359+
"supports_devtools": False,
360+
"supports_window_resize": False}
360361

361362
def check_crash(self, process, test):
362363
if not os.environ.get("MINIDUMP_STACKWALK", "") and self.stackwalk_binary:

Diff for: tools/wptrunner/wptrunner/executors/executormarionette.py

+8
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,7 @@ def __init__(self, logger, browser, server_config, timeout_multiplier=1,
914914
self.debug_test = debug_test
915915

916916
self.install_extensions = browser.extensions
917+
self.initial_window_size = None
917918

918919
self.original_pref_values = {}
919920

@@ -928,6 +929,10 @@ def setup(self, runner, protocol=None):
928929
addons.install(extension_path)
929930

930931
self.protocol.testharness.load_runner(self.last_environment["protocol"])
932+
try:
933+
self.initial_window_size = self.protocol.window.get_rect()
934+
except Exception:
935+
pass
931936

932937
def is_alive(self):
933938
return self.protocol.is_alive()
@@ -974,6 +979,9 @@ def do_testharness(self, protocol, url, timeout):
974979

975980
test_window = protocol.base.create_window()
976981
self.protocol.base.set_window(test_window)
982+
# Restore the window to the initial position
983+
if self.browser.supports_window_resize and self.initial_window_size:
984+
self.protocol.window.set_rect(self.initial_window_size)
977985

978986
if self.debug_test and self.browser.supports_devtools:
979987
self.protocol.debug.load_devtools()

Diff for: tools/wptrunner/wptrunner/executors/executorwebdriver.py

+15
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,14 @@ def __init__(self, logger, browser, server_config, timeout_multiplier=1,
996996

997997
self.close_after_done = close_after_done
998998
self.cleanup_after_test = cleanup_after_test
999+
self.initial_window_size = None
1000+
1001+
def setup(self, runner, protocol=None):
1002+
super().setup(runner, protocol)
1003+
try:
1004+
self.initial_window_size = self.protocol.window.get_rect()
1005+
except Exception:
1006+
pass
9991007

10001008
def on_environment_change(self, new_environment):
10011009
if new_environment["protocol"] != self.last_environment["protocol"]:
@@ -1025,6 +1033,13 @@ def do_testharness(self, protocol, url, timeout):
10251033
# The previous test may not have closed its old windows (if something
10261034
# went wrong or if cleanup_after_test was False), so clean up here.
10271035
protocol.testharness.close_old_windows()
1036+
# Restore the window to the initial position
1037+
if self.initial_window_size:
1038+
try:
1039+
self.protocol.window.set_rect(self.initial_window_size)
1040+
except Exception:
1041+
pass
1042+
10281043
raw_results = self.run_testdriver(protocol, url, timeout)
10291044
extra = {}
10301045
if counters := self._check_for_leaks(protocol):

0 commit comments

Comments
 (0)