Run only one measurement at a time 2.0.5 - #89
Draft
Victor1Ja wants to merge 1 commit into
Draft
Conversation
Nothing stopped two ndt7 runs from overlapping. Two tests sharing the link measure each other's traffic as congestion, so both report speeds that are too low. Reproduced by hand: two manual tests plus the post-registration one, all three at once. - MeasurementClientService.runTest() is now single-flight. A request that arrives while a run is in flight is dropped rather than queued: a measurement that starts late is worth less than the one already running, and queueing would only move the overlap later. Every trigger goes through here, so it covers the manual button, the post-registration test and both scheduled paths. - The test button is disabled while a run is live. startNDT() also returns before touching the UI: everything after that point reset progress and cleared the timers of the test that was still running, which made the app look stuck and invited another tap. - A scheduled slot whose turn comes up during another measurement leaves its semaphore untouched and returns. The 60s tick retries it, so the slot waits for the link instead of being lost, and it costs no retry attempt and no backoff. Assisted-by: Claude Opus 5
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.
Nothing stopped two ndt7 runs from overlapping. Two tests sharing the link measure each other's traffic as congestion, so both report speeds that are too low.
Reproduced by hand on the test build: two manual tests plus the post-registration one, all three running at once.
What changed
One measurement at a time —
MeasurementClientService.runTest()is now single-flight.firsttest, the scheduled slots and the startup test. A guard on the button alone would not have caught thefirsttest, which comes from another call path.finally, so a failed test frees it too.The test button is disabled while a run is live.
startNDT()also returns before touching the UI. Everything after that point resetprogressto 0 and cleared the timers of the test still running — which is what made the app look stuck and invited another tap.progressValue === 0 || 100, which is why the bug looked intermittent. At the very start progress is 0, so the second tap went through.A scheduled slot waits instead of being lost.
decide()leaves the semaphore untouched and returns.Tests
9 new specs:
firstblocks the manual path; a scheduled run is held off during a manual onetestRunning$transitionsVerification
tsc --noEmit -p tsconfig.spec.jsonclean andng buildclean.ng testcannot run in this checkout:@angular-devkit/build-angular@19.2.17is installed but itsbuilders.jsonis missing — the file its ownpackage.jsondeclares under"builders"— so the CLI cannot resolve the karma target. Unrelated to this change; worth a look before merging so CI is not the first place it runs.Not included
The startup test still has two known ways to schedule on top of a slow run: its 10-minute guard window is shorter than a slow ndt7, and the day is only marked after a successful run, so a failure leaves it to be rescheduled on the next tick. Both are far rarer than the overlap fixed here and are better handled together with the retry policy.