test(e2e): bound the waits in the disaster recovery test - #668
Open
MRmarioruci wants to merge 1 commit into
Open
MRmarioruci wants to merge 1 commit into
MRmarioruci wants to merge 1 commit into
Conversation
The test shares one budget across a long flow and contained two loops that could not fail on their own: one polling the station module hash, one polling for the restored balance. Whichever step happened to be slowest consumed the budget, and the failure was reported as a timeout inside whichever wait the test was sitting in, which is why the reported line moves between steps across runs. Both loops become bounded polls with their own timeouts and messages, so a step that does not complete fails against that step and reports the value it last read. The entry point into the recovery page is also waited for rather than clicked immediately, since it only appears once the wallet has noticed the station is uninstalled.
|
✅ No security or compliance issues detected. Reviewed everything up to 0325922. Security OverviewDetected Code Changes
|
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
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.
Problem
can recover uninstalled stationfails often onmain, and the reported line moves between steps across runs: 55, 71, 72 and 78 in recent runs. That is one symptom, not four.The test shares a single 600s budget across register, create station, top up, install a custom wasm, recover, verify balance. Inside that budget sat two loops that could not fail on their own:
installCustomWasmpolled the module hash in awhileloop paced bywaitForTimeout(1000)while (true)paced bywaitForTimeout(5000)Neither had an exit other than success, so whichever step was slowest on a given run consumed the remaining budget, and Playwright reported the failure as a timeout inside whichever
waitForTimeoutit happened to be in. The error therefore pointed at a step that was often not the slow one, and never said what it had actually observed.Changes
Both loops become
expect.pollwith their own timeout and message, sized to fit inside the existing 600s budget so the phase bound is what fires rather than the shared one. A step that does not complete now fails against that step and reports the value it last read, for example the balance it actually saw instead ofwaitForTimeout exceeded. The entry point into the recovery page is waited for before being clicked, since it only appears once the wallet has noticed the station is uninstalled, and the locator is narrowed to the first match.Note
This is a diagnosis change, not a claim that the test will now pass. If the balance genuinely does not return after a recovery, this makes that visible and attributable instead of hiding it behind a shared timeout. The test does currently pass on some runs, which points at slowness rather than a hard break, but the current failure mode is not specific enough to say so with confidence.