fix(test-optimization): jest instrumentation applies auto test retries on top of other retry mechanisms#7251
Conversation
Overall package sizeSelf size: 4.41 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 2.0.0 | 68.46 kB | 797.03 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7251 +/- ##
==========================================
- Coverage 86.25% 78.87% -7.39%
==========================================
Files 513 392 -121
Lines 22054 18166 -3888
==========================================
- Hits 19023 14328 -4695
- Misses 3031 3838 +807 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
BenchmarksBenchmark execution time: 2026-01-20 15:17:38 Comparing candidate commit 14f60c8 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 232 metrics, 28 unstable metrics. |
juan-fernandez
left a comment
juan-fernandez
left a comment
There was a problem hiding this comment.
👏 nice! Only minor comments, otherwise LGTM
integration-tests/jest/jest.spec.js
Outdated
| }) | ||
|
|
||
| context('early flake detection', () => { | ||
| it('takes precedence over flaky test retries for new tests', (done) => { |
There was a problem hiding this comment.
could we try to make the test async instead of using done? I've been trying to get rid of done
integration-tests/jest/jest.spec.js
Outdated
| } | ||
| ) |
There was a problem hiding this comment.
nitpick
| } | |
| ) | |
| }) |
There was a problem hiding this comment.
I was surprised to see we don't have code formatting as part of linting. Fixed it manually
| eventsPromise | ||
| ]) | ||
| }) | ||
|
|
There was a problem hiding this comment.
should we also check that attempt to fix takes precedence over retries triggered by impacted_tests_enabled: true?
There was a problem hiding this comment.
Good point, added a test for that too
| const isAttemptToFix = this.isTestManagementTestsEnabled && | ||
| this.testManagementTestsForThisSuite?.attemptToFix?.includes(testFullName) | ||
| if ( | ||
| this.isTestManagementTestsEnabled && |
There was a problem hiding this comment.
nitpick: this is redundant as it's already a requirement for isAttemptToFix
| this.isTestManagementTestsEnabled && |
There was a problem hiding this comment.
Good catch, removed that part of the condition
…s on top of other retry mechanisms
calvinbayer
dismissed stale reviews from juan-fernandez and anmarchenko
via
14f60c8
calvinbayer
force-pushed
the
calvin.bayer/fix/retry-mechanism-priority-order
branch
from
13aae60 to
14f60c8
Compare
calvinbayer
deleted the
calvin.bayer/fix/retry-mechanism-priority-order
branch
…s on top of other retry mechanisms (#7251) * fix(test-optimization): jest instrumentation applies auto test retries on top of other retry mechanisms * test: refactor test to be asynchronous and not rely on done callback * style: format closing parenthesis * refactor: remove redundant condition * test: add test to check atf takes precedence over efd for impacted tests
…s on top of other retry mechanisms (#7251) * fix(test-optimization): jest instrumentation applies auto test retries on top of other retry mechanisms * test: refactor test to be asynchronous and not rely on done callback * style: format closing parenthesis * refactor: remove redundant condition * test: add test to check atf takes precedence over efd for impacted tests
3 participants
What does this PR do?
Introduce an order of priority for applying retry mechanisms:
This change aligns the behavior of the Jest instrumentation with how other tracers do it. It also decreases the number of retries and thus the performance overhead introduced by instrumenting tests with Test Optimization. E.g., before this change, if both EFD and ATR were enabled, a new failing test would be executed 66 times:
(1 execution + 10 EFD retries) * (1 execution + 5 ATR retries). After this change, ATR retries will not be applied anymore and this test is only executed 11 times as expected.Motivation
Working on adding the
@test.final_statustag had unexpected side effects with multiple retry mechanisms being applied.