Refactor wpt test runner as a prerequisite for leak checking for web-platform-tests

RESOLVED FIXED

Status

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

People

(Reporter: jgraham, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

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

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
Because this should have been done some time ago.

One problem is how to deal with tests from upstream that leak on import. We can't delay the import in order to fix them, so we might need to add a metadata mechansim for skipping leak checking during those tests, or similar.
(Reporter)

Comment 1

2 years ago
Created attachment 8829502 [details] [diff] [review]
wip squashed patch

Added the extra bits I had on try to help people trying to reproduce issues.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

2 years ago
mozreview-review
Comment on attachment 8831721 [details]
Bug 1333114 - Enable leak checking for web-platform-tests,

https://reviewboard.mozilla.org/r/108268/#review109566

::: testing/web-platform/README.md:176
(Diff revision 1)
>                  dom.caches.enabled:true]
>  
> +Disabiling Leak Checks
> +----------------------
> +
> +When a test is imported that leak, it may be necessary to temporarily

"that leaks"

Comment 7

2 years ago
mozreview-review
Comment on attachment 8831720 [details]
Bug 1333114 - Refactor wpt TestRunner,

https://reviewboard.mozilla.org/r/108266/#review109562

::: testing/web-platform/harness/wptrunner/testrunner.py:114
(Diff revision 1)
> +            self.send_message("error",
> +                              "Timed out when waiting for %s" %
> +                              (command,))
> +            raise
> +
> +        if command != command:

What?

::: testing/web-platform/harness/wptrunner/testrunner.py:216
(Diff revision 1)
> +                succeeded = True
> +                self.started = True
> +
> +        # This has to happen after the lock is released
> +        if not succeeded:
> +            return False

What's the point of this `if`?
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Reporter)

Updated

a year ago
Attachment #8831719 - Attachment is obsolete: true

Comment 10

a year ago
mozreview-review
Comment on attachment 8831720 [details]
Bug 1333114 - Refactor wpt TestRunner,

https://reviewboard.mozilla.org/r/108266/#review114522

::: testing/web-platform/harness/wptrunner/testrunner.py:54
(Diff revision 2)
>          """Class implementing the main loop for running tests.
>  
>          This class delegates the job of actually running a test to the executor
>          that is passed in.
>  
>          :param test_queue: subprocess.Queue containing the tests to run

Remove

::: testing/web-platform/harness/wptrunner/testrunner.py:613
(Diff revision 2)
>      def error(self, message):
>          self.logger.error(message)
>          self.restart_runner()
>  
> +    def stop_runner(self, force=False):
> +        """Stop the TestRunner and the Firefox binary."""

Is this code specific to Fx?
Attachment #8831720 - Flags: review?(Ms2ger) → review+
(Reporter)

Updated

a year ago
Depends on: 1341666
(Reporter)

Updated

a year ago
Depends on: 1341673
(Reporter)

Updated

a year ago
Depends on: 1341685
(Reporter)

Updated

a year ago
Depends on: 1341688
Comment hidden (mozreview-request)
(Reporter)

Updated

a year ago
Attachment #8831721 - Attachment is obsolete: true
Attachment #8831721 - Flags: review?(Ms2ger)
Comment hidden (mozreview-request)
(Reporter)

Updated

a year ago
Keywords: leave-open

Comment 16

a year ago
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/a0e6a0c4712a
Refactor wpt TestRunner, r=automatedtester,Ms2ger
(Reporter)

Comment 18

a year ago
I poisoned the well on this bug by using it to land a prerequisite via mozreview. So created a new meta bug for the whole feature and moved the dependencies there.
Blocks: 1352355
No longer depends on: 1333109, 1341673, 1341685, 1341688, 1333106, 1333112, 1341666
Summary: Enable leak checking for web-platform-tests → Refactor wpt test runner as a prerequisite for leak checking for web-platform-tests
(Reporter)

Updated

a year ago
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
(Reporter)

Updated

a year ago
No longer blocks: 1352355
(Reporter)

Updated

a year ago
Blocks: 1352355
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.