Try to close all open windows after each test

RESOLVED FIXED in Firefox 52

Status

Testing
web-platform-tests
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jgraham, Unassigned)

Tracking

unspecified
mozilla52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

To reduce the possibility of leaking environments.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

2 years ago
mozreview-review
Comment on attachment 8804178 [details]
Bug 1312675 - Try harder to close existing windows after each testharness test,

https://reviewboard.mozilla.org/r/88290/#review87310

Can we see a try run before landing this?

::: testing/web-platform/harness/wptrunner/executors/executormarionette.py:128
(Diff revision 3)
> +    def set_timeout(self, timeout):
> +        self.marionette.set_script_timeout(timeout * 1000)
> +        self.timeout = timeout

Document that the `timeout` argument takes seconds.

::: testing/web-platform/harness/wptrunner/executors/executormarionette.py:157
(Diff revision 3)
> +        try:
> +            handles.remove(self.runner_handle)
> +            runner_handle = self.runner_handle
> +        except ValueError:
> +            # The runner window probably changed id but we can restore it
> +            runner_handle = handles[0]

It’s not guaranteed by the WebDriver spec that the handles are sorted, but I guess that is the case with Marionette currently… in any case it shouldn’t matter what window you continue with as long as the others are cleaned up.

::: testing/web-platform/harness/wptrunner/executors/executormarionette.py:157
(Diff revision 3)
> +            runner_handle = handles[0]
> +            handles = handles[1:]

`runner_handle = handles.pop(0)` to do this in one atomic action.

::: testing/web-platform/harness/wptrunner/executors/executormarionette.py:162
(Diff revision 3)
> +            runner_handle = handles[0]
> +            handles = handles[1:]
> +
> +        for handle in handles:
> +            self.marionette.switch_to_window(handle)
> +            self.marionette.close()

FYI I’m currently working on a patch that returns the number of window handles on calling `close()`.
Attachment #8804178 - Flags: review?(ato) → review+

Comment 6

2 years ago
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b71ce57f747f
Try harder to close existing windows after each testharness test, r=ato

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b71ce57f747f
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.