Closed Bug 1557038 Opened 4 months ago Closed 3 months ago

Implement experimental Test Isolation Taskcluster Action

Categories

(Testing :: General, enhancement, P3)

enhancement

Tracking

(firefox69 fixed)

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: bc, Assigned: bc)

References

Details

Attachments

(6 files, 1 obsolete file)

Test Isolation is intended to investigate the effect of possibly isolating intermittent failing tests from the other tests. At this time, it is not intended to be a permanent feature.

It is to be implemented as a taskcluster action callback which when called for a job will determine the failing tests from the job's errorsummary log, then

  • trigger a job by cloning the original task.
  • trigger a job for each directory containing failing tests by cloning the original task, then setting MOZHARNESS_TEST_PATHS to the directory.
  • trigger a job for each failing test by cloning the original task, then setting MOZHARNESS_TEST_PATHS to the path to the test.

Initially it will be implemented for crashtest, jsreftest, mochitest, reftest and xpcshell-tests. web-platform-tests are currently excluded since the test id contained in the errorsummary log does not work with either MOZHARNESS_TEST_PATHS or as a command line argument.

Caveats:

  • xpcshell tests do not currently report crashes to the error summary log (Bug 1087144)
  • reftests do not currently report assertion count failures to the error summary log (Bug 1244739)
  • mochitests do not currently report assertion counts to the error summary log. See debug-mochitest-e10s-5 for an example.

An example try run

This PR just adds Run Isolation Test to the Job detail panel. I think it would be good to also add it to the classification pull down as n Isolate all selection so that sheriffs could pin several jobs and invoke isolate on all of them at once but wanted to get your opinion on the best way first.

This uses a prompt to get the number of times the test should be triggered.

ekyle: Aryx suggested I get your feedback on if this approach of reusing the task labels and changing the group and job symbols would affect activedata. See try run for an example in action.

Flags: needinfo?(klahnakoski)

bc: I do not understand the change you made, but it is very likely your change is "just data" to ActiveData, so nothing broke. I am seeing no problem with the ingestion of the example you gave me:

The tasks appear to be good: https://activedata.allizom.org/tools/query.html#query_id=X28Sm8U_
The unittests (that did run) appeared to pass: https://activedata.allizom.org/tools/query.html#query_id=hjkEsQAV

Flags: needinfo?(klahnakoski)
Keywords: leave-open
Pushed by bclary@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f4eb5cb790fc
Implement experimental Test Isolation Taskcluster Action, r=tomprince.
Attachment #9069955 - Attachment is obsolete: true

camd: I messed up my earlier pull request attempting to update it. I've closed it and pushed an updated one with the lint fixes and two additional changes squashed together. I don't have permission to request a reviewer. If you could go ahead and assign yourself as a reviewer I would really appreciate it.

Flags: needinfo?(cdawson)

gbrown: I added you as the reviewer for the two new patches. This bumps the maximum iterations to 100, and adds additional checks to eliminate bad test paths from the error summary log. The new pull request to treeherder also removed the ability to run isolation tests on jsreftests since it is not possible to specify the test path from the one found in the error summary log.

Try push with isolation tests for failing jobs

When it is not possible to determine the test path, the test isolation simply reruns the original job again. You can see this with the devtools leaks which are not reported to the error summary log in the same way as other test and for mochitest application timed out errors which are not reported to the error summary log at all.

Code has been reviewed and approved. Please let me know if you'd like me to merge or wait till someone else has reviewed the code.

Flags: needinfo?(cdawson)

Let me push my changes for the action to autoland this evening and we can merge the treeherder changes on Wednesday.

Pushed by bclary@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/88a9b7b37a81
Handle additional invalid errorsummary test values in isolate-test-failures action, r=gbrown.
https://hg.mozilla.org/integration/autoland/rev/9b7d0410ba13
increase maximum number of isolation tests to 100, r=gbrown.
Priority: -- → P3

This crashtest illustrates a bug in determining the test path for crashtests for id and it tests.

'MOZHARNESS_TEST_PATHS': '{"crashtest": ["file:///builds/worker/workspace/build/tests/reftest/tests/accessible/tests/crashtests"]}'
'MOZHARNESS_TEST_PATHS': '{"crashtest": ["file:///builds/worker/workspace/build/tests/reftest/tests/accessible/tests/crashtests/1402999.html"]}'
Flags: needinfo?(bob)

I ran a test of Stockwell Isolation bugs with the current implementation.

KWierso mentioned that it might be better to make these new jobs tier 3. A bc7 failure illustrates the current situation. All of the new isolation jobs appear as Tier 1 and will affect the unclassified count for sheriffs. Unless we want the Sheriffs to also classify the isolation tests, it would be better to make the new jobs not appear on in the sheriff's radar.

Aryx, jmaher: Ok with making the new isolation jobs Tier3?

Flags: needinfo?(aryx.bugmail)

I've gone ahead and made the new jobs tier 3.

Flags: needinfo?(aryx.bugmail)
  • Incorporated the regular expressions into a list.

  • Removed the ^ restriction in the path patterns since it is redundant when using match.

  • Added handling of other schemes such as view-source when it preceeds file or http schemes.

  • Removed the checking for initial slashes following the url scheme and removed the
    requirement for a trailing space.

  • Convert test path forward slashes to backslashes in Windows.

Flags: needinfo?(bob)
Keywords: leave-open

The most recent try run: https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=252311208&tier=1%2C2%2C3&revision=9b6939ddf0c13076cfed302c0afe890449d363d8

I had inadvertently removed one of the bad test path patterns during this run but the final patch includes it again.

Thank you. Tier 3 fits for this situation.

Pushed by bclary@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/89776a5688e3
[test isolation] update extraction of test path from errorsummary log, r=gbrown.
https://hg.mozilla.org/integration/autoland/rev/d1e530d3495e
[test isolation] change test isolation tasks to tier 3, r=gbrown.
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Blocks: 1561306
You need to log in before you can comment on or make changes to this bug.