Test repair agent Part 2: update the pulse listener - #6421
Open
evgenyrp wants to merge 15 commits into
Open
Conversation
Contributor
Author
|
This is preliminary as I'm still reviewing and testing myself. Also, it might change when the agent Part 1 lands. |
Brings in the build-repair action-task and push-age gates alongside the Treeherder-based rewrite of the listener's failure filtering. Conflict resolution: - regression.py: kept the Treeherder-backed regression gate (per-push status from Treeherder, ancestor chain from mozci) and added master's is_stale_push, whose param is named `project` to match the module. - test_regression.py / test_consumer.py: master's build-path tests, which use the new get_task seam, plus the test-path suite. - README.md: one pipeline covering both paths, including master's action-task and push-age steps. Two changes outside the conflicts: - consumer.py auto-merged cleanly but was left broken: master changed taskcluster.get_hg_revision to take a task definition rather than a task id, while _process_test still passed the id, which would raise AttributeError on every test-failure message. It now fetches the task once and passes it on. - The same two gates master added to builds now also apply to test failures, since the signature change was made for them and the reasoning carries over: a backfill is not a new failure, and a failure surfacing days after its push is not worth repairing. Both run before the ancestor walk, which can block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Current state of things: Over ~86 minutes: ~116 failures rejected at the Treeherder gate, 4 runs triggered: |
Collaborator
Looking at treeherder for this: https://treeherder.mozilla.org/jobs?repo=autoland&revision=0faca76bc591445f1de70a0081901faea33c8242&searchStr=mochitest-browser-chrome&selectedTaskRun=c2YUKOVWS8KEqC3kvBqYVg.0. Can you also look at the other 3 triggered runs to see if they were successfully finding what needed to be found? |
marco-c
reviewed
Jul 30, 2026
| 1: "not classified", | ||
| 2: "fixed by commit", | ||
| 3: "expected fail", | ||
| 4: "intermittent", |
Collaborator
There was a problem hiding this comment.
The "intermittent" classification is manual, not sure how quick usually sheriffs are at applying it. CC @Archaeopteryx
marco-c
reviewed
Jul 30, 2026
Comment on lines
+35
to
+42
| # Treeherder classifies a failing job shortly after we see the failure, so the | ||
| # gate waits for the job to be ingested before reading that verdict. | ||
| treeherder_ingest_poll_seconds: int = 30 | ||
| treeherder_ingest_max_wait_seconds: int = 240 | ||
| # How long to wait for a verdict once the job is ingested. Classification lands a | ||
| # few minutes after ingestion, and most test failures turn out to be intermittent | ||
| # or expected-fail, so waiting here rejects them before the ancestor walk. | ||
| treeherder_classification_wait_seconds: int = 300 |
Collaborator
There was a problem hiding this comment.
@Archaeopteryx if we wait this long and then the agent takes a few more minutes to analyze, do we risk sheriffs will already have run a backfill by the time the analysis is done?
Contributor
Author
There was a problem hiding this comment.
We do see that sometimes the analysis is late. It's likely due to this delay + the agent itself that builds Firefox, which takes 10 min. We can try improving it here or in a follow-up PR with other improvements.
Collaborator
There was a problem hiding this comment.
Can we cut the analysis short when it's clear what the culprit is from the patches and avoid building Firefox? At least in the first iteration, we don't need to share the fix with the sheriffs, the sheriffs will always backout.
Contributor
Author
This was a dry run. I added treeherder URLs to the logs for easier inspection. I'm going to deploy it with a daily limit and then we can look at what actual agent runs look like. |
Contributor
Author
|
Based on testing, it's mostly working, but there are known issues that we can address as a follow-up:
|
evgenyrp
changed the title
evgenyrp
marked this pull request as ready for review
August 4, 2026 20:43
Collaborator
|
The main thing to fix is the lateness. Analyzing already known intermittents is fine, as long as the analysis is correct (it's just a cost that we can avoid, but not a blocker). |
marco-c
reviewed
Aug 5, 2026
| run_try_push: bool = False | ||
| model: str | None = None | ||
| max_turns: int | None = None | ||
| # Treeherder classifies a failing job shortly after we see the failure, so the |
Collaborator
There was a problem hiding this comment.
Suggested change
| # Treeherder classifies a failing job shortly after we see the failure, so the | |
| # Sheriffs classify a failing job shortly after we see the failure, so the |
marco-c
reviewed
Aug 5, 2026
| each failing job while parsing its log -- a cross-push analysis of per-manifest | ||
| pass rates, refined afterwards by sheriffs and by mozci's autoclassifier. Reading | ||
| that verdict is cheaper and broader than judging intermittency ourselves: it covers | ||
| every harness, where the tests.firefox.dev timings datasets it replaces published |
Collaborator
There was a problem hiding this comment.
We should not mention tests.firefox.dev datasets it replaces since it never was in the code (it was in your local code I guess)
marco-c
reviewed
Aug 5, 2026
| @@ -0,0 +1,312 @@ | |||
| """Treeherder classification gate for test-repair. | |||
|
|
|||
| Treeherder ingests the same Taskcluster failures the listener sees and classifies | |||
Collaborator
There was a problem hiding this comment.
treeherder doesn't classify, it's mostly sheriffs doing it manually
marco-c
reviewed
Aug 5, 2026
| # a lock. | ||
| # Taskcluster ``kind`` tags that denote test tasks (vs build tasks). | ||
| # | ||
| # ``source-test`` is deliberately absent. It is not a Firefox test harness: on a |
Collaborator
There was a problem hiding this comment.
Some actually are (python tests), but we can skip these for now.
They are mostly linting tasks. We can still repair them, but they are closer to builds than tests.
marco-c
reviewed
Aug 5, 2026
| # recent push its 22 task types were all mozlint, shadow-scheduler and file-metadata | ||
| # checks. None can be repaired by the agent's method (clone, build Firefox, re-run | ||
| # the failing test with mach), so routing them here only spends runs. | ||
| TEST_KINDS = {"test", "mochitest", "web-platform-tests"} |
marco-c
reviewed
Aug 5, 2026
| # classified intermittent), and the agent's method of re-running a failing | ||
| # manifest with mach does not apply to them. Checked on the label, so it costs | ||
| # nothing. | ||
| if is_no_groups_suite(label): |
Collaborator
There was a problem hiding this comment.
We should still include them, perhaps as a follow-up
marco-c
reviewed
Aug 5, 2026
| return None | ||
|
|
||
| # Cheapest gate first: it rules out intermittents and infra failures for every | ||
| # harness before any group resolution or ancestor walking. Treeherder classifies |
Collaborator
There was a problem hiding this comment.
Suggested change
| # harness before any group resolution or ancestor walking. Treeherder classifies | |
| # harness before any group resolution or ancestor walking. Sheriffs classify |
marco-c
reviewed
Aug 5, 2026
| - Tasks scheduled by an **action task** rather than by the push: `extra.parent` points | ||
| at the decision task (= the task group) for everything the push scheduled, and at the | ||
| action-callback task for a backfill or retrigger. | ||
| - Pushes that landed more than `MAX_PUSH_AGE_HOURS` ago (default 24). A failure can |
marco-c
reviewed
Aug 5, 2026
| surface long after its push, and by then the push has been superseded. | ||
| - Test suites that report no manifests (gtest, junit, talos, raptor, jittest, ... — | ||
| mozci's `is_no_groups_suite`). Their failures are overwhelmingly crashes and | ||
| timeouts, and the agent's method of re-running a failing manifest does not apply. |
Collaborator
There was a problem hiding this comment.
It could still find the culprit even if it is not able to reproduce the issue
marco-c
reviewed
Aug 5, 2026
| build to finish first. | ||
| - _Test:_ first drop whatever Treeherder judges not to be a new regression | ||
| (intermittent, infra, expected-fail, fixed-by-commit). Treeherder ingests a minute or | ||
| so behind us and classifies well after that — a median of ~19 minutes past the end of |
Collaborator
There was a problem hiding this comment.
Suggested change
| so behind us and classifies well after that — a median of ~19 minutes past the end of | |
| so behind us and sheriffs classify well after that — a median of ~19 minutes past the end of |
marco-c
reviewed
Aug 5, 2026
| build to finish first. | ||
| - _Test:_ first drop whatever Treeherder judges not to be a new regression | ||
| (intermittent, infra, expected-fail, fixed-by-commit). Treeherder ingests a minute or | ||
| so behind us and classifies well after that — a median of ~19 minutes past the end of |
marco-c
reviewed
Aug 5, 2026
| any ancestor walked. What survives is | ||
| narrowed to the groups that are new for this task's own configuration (platform and | ||
| build option), then Treeherder is asked once more, since a verdict can still land | ||
| while that walk runs. The run carries only the task id. |
Collaborator
There was a problem hiding this comment.
Could you add some comment about how the ancestor walk works for test, since it is different than for build?
marco-c
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Redeployed for testing after running locally for 18 hours. It did catch some real issues but also some intermittent issues.
depends on #6375
fixes #6399