Repository navigation
Keep the configured interval test's update check from running before its verify - #161
Draft
jwrosewell wants to merge 1 commit into
Draft
jwrosewell wants to merge 1 commit into
jwrosewell wants to merge 1 commit into
Conversation
…its verify With no polling interval, registerDataFile schedules the check after a random number of whole seconds below 600, which is 0 about once in 600 registrations. The test's future factory runs the check on a real executor, and a check that finds no update schedules again, so the test then saw two schedule calls where it verifies one. That failed Ubuntu_Java_17 in full build 43667464820. The test now has the factory return a future that never runs, as it checks only the delay.
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.
Tests only.
DataUpdateServiceTests.DataUpdateService_Register_AutoUpdateConfiguredIntervalfails about once in 600 runs, and it failedUbuntu_Java_17in full build 43667464820 of #156, which changes nothing inpipeline.engines. This fixes the cause rather than leaving it to a re-run, which is the rule for a test that fails on timing that #122 writes into AGENTS.md.The cause
The test registers a data file with automatic updates on and a polling interval of 0, then verifies that
futureFactory.schedulewas called once with a delay no longer than the randomisation maximum. With no polling interval, the delay is only the random part, a whole number of seconds below 600 (DataUpdateServiceDefault.java lines 736 to 761,random.nextInt(config.getMaxRandomisationSeconds())), so it is 0 about once in 600 registrations. The test class's future factory runs each scheduled task on a real executor (DataUpdateServiceTests.java lines 129 to 150), so a check scheduled at 0 runs straight away. A check that finds no update schedules again (DataUpdateServiceDefault.java lines 287 to 298), and the verify then sees two calls. The failure in 43667464820 shows exactly that, "Wanted 1 time ... But was 2 times", the second call fromcheckForUpdateat line 298.What changed
The test now has the future factory return a
ScheduledFuturethat never runs, as it checks only the delay passed toschedule. The other tests in the class schedule with a delay of at least the update's availability or the 30 minute polling default, so they cannot hit the same race. No package code changes.Tests
Test change 6f98bec3.
The red and green force the case the race needs, by setting the test's maximum randomisation to 1 second (so the delay is always 0) and waiting 2 seconds before the verify, plus a throwaway workflow, Java 17. Red 43674099493 ran that on
rcde74c8c and the test failed in all four of its parameter runs withTooManyActualInvocations("Wanted 1 time ... But was 67374 times", as each check at 0 schedules the next at 0). Green 43674088956 ran the same forcing on the head, andDataUpdateServiceTestspassed 132 of 132.Full build
Under way, without macOS under the 9 November decision. This section is replaced with the run, its result and the head it built when it ends.
What it releases
Nothing. The change is under
src/test, which the release plan leaves out (ci/README.md).Base branch
rcat de74c8c.