1144194 - Mochitest should only parse test manifests once

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Andrew Halberstadt [:ahal]]

Currently mochitest parses the test manifests at least three times (and once per directory in the event of --run-by-dir). This is both inefficient and can cause obscure errors where the list of tests differs slightly from one call to another.

Instead the manifests should just be parsed once and the resulting list of tests saved for the future.

Comment 1

[Andrew Halberstadt [:ahal]]

The function that does the parsing is 'getActiveTests':
https://dxr.mozilla.org/mozilla-central/search?tree=mozilla-central&q=getActiveTests&redirect=true

This function gets called by 'getTestsToRun', 'getDirectories' and 'buildTestPath' which themselves can get called multiple times. Each time it is invoked a whole new manifest object is instantiated.

Comment 2

[Andrew Halberstadt [:ahal]]

This would also fix a bug I found.

When using --manifest with --run-by-dir, mochitest will run the incorrect set of tests. At first getActiveTests() uses options.manifestFile correctly, and returns the correct set of tests. But in buildTestPath (as well as initializeLooping) we change options.manifestFile to something else [1]. Normally this is fine since we've already called getActiveTests(). But with --run-by-dir enabled, getActiveTests() is called once per directory [2]. Now options.manifestFile no longer points to the original manifest we passed in and we'll fall back to using the master manifest [3] instead (completely ignoring --manifest).

In automation we don't use --manifest, so this isn't a problem. But we use it with mach [4]. Unfortunately there's no failure, just the wrong set of tests subtly getting run.


[1] https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#776
[2] https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#2058
[3] https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#2501
[4] https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/mach_commands.py#432

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: MozReview Request: bz://1144194/ahal

/r/5597 - Bug 1144194 - Only parse test manifests once in mochitest, r=jmaher

Pull down this commit:

hg pull review -r 3353402fa0bfa27afbbfad748ead180735605158

Comment 4

[Andrew Halberstadt [:ahal]]

Comment on attachment 8579327 [details]
MozReview Request: bz://1144194/ahal

/r/5597 - Bug 1144194 - Only parse test manifests once in mochitest, r=jmaher

Pull down this commit:

hg pull review -r 3353402fa0bfa27afbbfad748ead180735605158

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8579327 [details]
MozReview Request: bz://1144194/ahal

https://reviewboard.mozilla.org/r/5595/#review4623

Ship It!

Comment 6

[Andrew Halberstadt [:ahal]]

Try looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2c7d5841f7f

Landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0918170249a

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e0918170249a

Comment 9

[Andrew Halberstadt [:ahal]]

Attachment: MozReview Request: Bug 1144194 - Only parse test manifests once in mochitest, r=jmaher