Closed Bug 1144194 Opened 7 years ago Closed 7 years ago

Mochitest should only parse test manifests once

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(firefox39 fixed)

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: ahal, Assigned: ahal)

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
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
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Attached file MozReview Request: bz://1144194/ahal (obsolete) —
/r/5597 - Bug 1144194 - Only parse test manifests once in mochitest, r=jmaher

Pull down this commit:

hg pull review -r 3353402fa0bfa27afbbfad748ead180735605158
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
Attachment #8579327 - Flags: review?(jmaher)
Comment on attachment 8579327 [details]
MozReview Request: bz://1144194/ahal

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

Ship It!
Attachment #8579327 - Flags: review?(jmaher) → review+
https://hg.mozilla.org/mozilla-central/rev/e0918170249a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Attachment #8579327 - Attachment is obsolete: true
Attachment #8619783 - Flags: review+
You need to log in before you can comment on or make changes to this bug.