fix: ensure that current retry is maintained properly when tests fail prior to top changing#32888
Conversation
… prior to top changing
There was a problem hiding this comment.
Bug: Guard Against Null Runnable in currentRetry Access
The currentRetry getter accesses this.cy.state('runnable').ctx without checking if this.cy.state('runnable') is null/undefined first. This will throw a TypeError if runnable is nullish. The currentTest getter above (lines 930-948) properly handles this case with a null check. The getter should be:
get currentRetry (): number {
const r = this.cy.state('runnable')
if (!r) {
return 0
}
const ctx = r.ctx
return ctx?.currentTest?._currentRetry || ctx?.test?._currentRetry || 0
}This is particularly relevant since preserveRunState now calls this.currentRetry (line 864) during cross-origin navigations, where the runnable state might not be guaranteed to be set.
packages/driver/src/cypress.ts#L949-L954
cypress/packages/driver/src/cypress.ts
Lines 949 to 954 in c991b8c
cypress
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress
|
| Branch Review |
ryanm/fix/retries-before-top-change
|
| Run status |
|
| Run duration | 19m 17s |
| Commit |
|
| Committer | Ryan Manuel |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
9
|
|
|
1098
|
|
|
4
|
|
|
26530
|
| View all changes introduced in this branch ↗︎ | |
Warning
Partial Report: The results for the Application Quality reports may be incomplete.
UI Coverage
45.48%
|
|
|---|---|
|
|
188
|
|
|
161
|
Accessibility
97.98%
|
|
|---|---|
|
|
4 critical
8 serious
2 moderate
2 minor
|
|
|
101
|
Added changelog entry for version 15.6.1 with a bugfix.
Co-authored-by: Bill Glesias <bglesias@gmail.com>
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
When tests fail and then retry and top needs to change on a retry, we were previously not saving the state of the retry, so from the test runner's perspective you might end up seeing something like "Attempt 1 of 3", "Attempt 1 of 3", Success instead of "Attempt 1 of 3", "Attempt 2 of 3", Success. We fix this issue by saving the retry (in addition to the test id) when we save state right before a top change and then rehydrate that when we resume. This ensures that the retry number remains consistent.
In addition, when this happens, we end up wiping the entire test in terms of what is sent to test replay instead of only the retry that triggered the top change. We fix this by sending the retry on a resert to protocol and protocol only deletes the test/retry combination from the database.
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?Note
Ensure the current test retry is saved/restored across top changes and propagated to protocol reset, with tests and types updated.
currentRetrytoRunStateincypress.preserveRunStateand resuming viaCypress.runner.resumeAtTest(id, currentRetry, emissions).runner.resumeAtTestto acceptcurrentRetryand settest._currentRetrywhen resuming.resetTest(testId, currentRetry?)and invoke it withcurrentRetryfrom cached run state insocket-base.ts.ProtocolManager.resetTestsignature and underlying types.currentRetry?: number | nulltoRunStateand to protocol interfaces.Written by Cursor Bugbot for commit 664916c. This will update automatically on new commits. Configure here.