1707876 - Support leak parsing for wdspec tests

Status: RESOLVED FIXED

(Testing :: geckodriver, enhancement, P3)

Description

[James Graham [:jgraham]]

We currently run wdspec in asan builds, but in the case that there are leaks we don't correctly parse the output so the job will fail (because treeherder will detect the LSAN failures) but we won't be able to annotate the leaks in metadata, or run mozleak processing.

To fix this we need to run the output from geckodriver through the OutputHandler we define for Firefox output, which includes code to parse the output for LSAN failures.

Comment 1

[James Graham [:jgraham]]

Attachment: Bug 1707876 - Use Firefox output handler for wdspec tests, r=whimboo

To allow parsing the LSAN output from wdspec tests we need to run
the output from geckodriver through the OutputHandler defined for
Firefox.

To do this we define a generic OutputHandler class which can be used
in all situations where there is a requirement to read output from a
subprocess. Because most browser implementations are based on the
WebDriverServer classes, we associate an output handler class with the
WebDriverServer which handles the common case for other browsers and
the wdspec case for Firefox.

Because for WebDriver tests the WebDriverServer is initalized from the
executor process, we need to wire the relevant handler options through
to the executor; for most cases this doesn't change anything, but for
Firefox this includes all the options related to leak checking which
requrie reading the process output.

Some complexity comes from the fact that we have a defered
initalization flow for Firefox in which we're able to preload Firefox
instances before we know which tests they will run. This requires the
OutputHandler startup to be multi-phase; initially we create a
handler, then we pass it down to the mozprocess.ProcessHandler so it
recieves output, then we associate the process pid with the handler,
then finally once we know what configuration we're running, we know
what processing will be applied, so we stop buffering the output and
start emitting it to the logger. We also have a lifecycle method that
must be called when the process is terminated; this is where any
bulk processsing of the complete output (e.g. LSAN messages) occurs.

Comment 2

[James Graham [:jgraham]]

Attachment: Bug 1707876 - Allow testrunner processes to use a full logger, r=whimboo

Previously we used a special MessageLogger class for logging in
TestRunner processes. This was only set up to allow normal logging
messages (debug/info/etc.). But for wdspec tests in Firefox we're
reading the geckodriver output in the TestRunner thread, so require
the corresponding log actions. The simplest way to do this is to
create an actual StructuredLogger in the child process and use a
handler on that logger to proxy the messages back to the parent, as
before.

Comment 3

[James Graham [:jgraham]]

Attachment: Bug 1707876 - Allow list newly detected LSAN failures, r=#webdriver-reviewers

Comment 4

[James Graham [:jgraham]]

Try results started looking reasonable: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=I8eW4_ijRE23Bh2oOZxpdw.0&tier=1%2C2%2C3&revision=7acbef0685f3c6ac983df86c6ae38fecd60eb8fe
Upstream PR to run the CI there: https://github.com/web-platform-tests/wpt/pull/28729

Comment 5

[James Graham [:jgraham]]

Attachment: Bug 1707876 - Make the specification of wdspec Browser class explicit, r=whimboo

Previously we had a small hack in wptrunner.py to override the browser
class with NullBrowser for all the cases where the wdspec test type
was being run. Apart from being a bit surprising, this doesn't allow
us to use different Browser classes per product, which can be useful
where we're associating data with the Browser instance related to
configuration rather than just with the mechanics of launching the
process.

To remove this hack, allow the "browser" key in a wptrunner
variable be a dictionary mapping test type to browser class name, with
None used as a key for the default class.

Comment 6

[James Graham [:jgraham]]

Attachment: Bug 1707876 - Add a FirefoxWdSpecBrowser type with test-specific browser settings, r=whimboo

This is needed by the FirefoxOutputHandler to process the lsan output with the correct
per-directory list of allowed failures

Comment 7

[James Graham [:jgraham]]

Attachment: Bug 1707876 - Remove browsers.base.inherit and remaining usage, r=whimboo

This was designed to allow reusing configuration across browsers, but
in practice it's only used once and is adding additional complexity
that isn't merited by the amount of use.

Comment 8

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

Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.

Comment 9

[Dorel Luca [:dluca]]

Backed out 14 changesets (bug 1686707, bug 1707876, bug 1678044) for Spidermonkey failure. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=340215481&repo=autoland&lineNumber=45654

Backout:
https://hg.mozilla.org/integration/autoland/rev/532f607c9ca037bdfc9d671a7447b28b19c05c77

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f796d42723fd
Use Firefox output handler for wdspec tests, r=whimboo
https://hg.mozilla.org/integration/autoland/rev/79ced8bb0b5f
Allow testrunner processes to use a full logger, r=whimboo
https://hg.mozilla.org/integration/autoland/rev/85dd729149c9
Allow list newly detected LSAN failures, r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/f0abe77f6bb9
Make the specification of wdspec Browser class explicit, r=whimboo
https://hg.mozilla.org/integration/autoland/rev/bc88cd35a55d
Add a FirefoxWdSpecBrowser type with test-specific browser settings, r=whimboo
https://hg.mozilla.org/integration/autoland/rev/55869e7f350c
Remove browsers.base.inherit and remaining useage, r=whimboo

Comment 11

[James Graham [:jgraham]]

Attachment: Bug 1707876 - Update jstests.py for wpt API change, r=whimboo

Depends on D113670

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/613b932002d5
Use Firefox output handler for wdspec tests, r=whimboo
https://hg.mozilla.org/integration/autoland/rev/6161d2be653b
Allow testrunner processes to use a full logger, r=whimboo
https://hg.mozilla.org/integration/autoland/rev/e592afd9715e
Allow list newly detected LSAN failures, r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/6ca6831327a0
Make the specification of wdspec Browser class explicit, r=whimboo
https://hg.mozilla.org/integration/autoland/rev/b1100fbffe9f
Add a FirefoxWdSpecBrowser type with test-specific browser settings, r=whimboo
https://hg.mozilla.org/integration/autoland/rev/b8ab0d84b8a7
Remove browsers.base.inherit and remaining usage, r=whimboo

Comment 13

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/613b932002d5
https://hg.mozilla.org/mozilla-central/rev/6161d2be653b
https://hg.mozilla.org/mozilla-central/rev/e592afd9715e
https://hg.mozilla.org/mozilla-central/rev/6ca6831327a0
https://hg.mozilla.org/mozilla-central/rev/b1100fbffe9f
https://hg.mozilla.org/mozilla-central/rev/b8ab0d84b8a7

Comment 14

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

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

Comment 15

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

Upstream PR merged by moz-wptsync-bot