Closed Bug 1707876 Opened 4 years ago Closed 4 years ago

Support leak parsing for wdspec tests

Categories

(Testing :: geckodriver, enhancement, P3)

Default
enhancement

Tracking

(firefox90 fixed)

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: jgraham, Assigned: jgraham)

References

Details

Attachments

(6 files, 1 obsolete file)

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.

Priority: -- → P3

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.

Assignee: nobody → james
Status: NEW → ASSIGNED

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.

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.

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

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.

Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.
Pushed by james@hoppipolla.co.uk: 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
Attachment #9218642 - Attachment description: Bug 1707876 - Use Firefox output handler for wdspec tests, → Bug 1707876 - Use Firefox output handler for wdspec tests, r=whimboo
Attachment #9218643 - Attachment description: Bug 1707876 - Allow testrunner processes to use a full logger, → Bug 1707876 - Allow testrunner processes to use a full logger, r=whimboo
Attachment #9218644 - Attachment description: Bug 1707876 - Allow list newly detected LSAN failures, → Bug 1707876 - Allow list newly detected LSAN failures, r=#webdriver-reviewers
Attachment #9218964 - Attachment description: Bug 1707876 - Make the specification of wdspec Browser class explicit, → Bug 1707876 - Make the specification of wdspec Browser class explicit, r=whimboo
Attachment #9218965 - Attachment description: Bug 1707876 - Add a FirefoxWdSpecBrowser type with test-specific browser settings, → Bug 1707876 - Add a FirefoxWdSpecBrowser type with test-specific browser settings, r=whimboo
Attachment #9218966 - Attachment description: Bug 1707876 - Remove browsers.base.inherit and remaining useage, → Bug 1707876 - Remove browsers.base.inherit and remaining useage, r=whimboo

Depends on D113670

Attachment #9218966 - Attachment description: Bug 1707876 - Remove browsers.base.inherit and remaining useage, r=whimboo → Bug 1707876 - Remove browsers.base.inherit and remaining usage, r=whimboo
Attachment #9222773 - Attachment is obsolete: true
Pushed by james@hoppipolla.co.uk: 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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29075 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot
Flags: needinfo?(james)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: