The data update test can see one schedule when a check runs before its future is stored #70

Problem

DataUpdateServiceTests.DataUpdateService_UpdateFromFile_FileUpdated can fail because the data update service schedules its second check only some of the time. It failed in the full pull request build of #68 on MacOS_X64_Java_11 (run 42455459082, attempt 1, job 128653447065). #68 does not change pipeline.engines.

DataUpdateService_UpdateFromFile_FileUpdated[0: Test with autoUpdateEnabled=true, setEngineNull=false ]
org.mockito.exceptions.verification.TooFewActualInvocations:
futureFactory.schedule(<any java.lang.Runnable>, <any long>);
Wanted 2 times:
-> at ...DataUpdateServiceTests.DataUpdateService_UpdateFromFile_FileUpdated(DataUpdateServiceTests.java:490)
But was 1 time:
-> at ...DataUpdateServiceDefault.registerDataFile(DataUpdateServiceDefault.java:437)

What the code does

Line numbers are for rc at 22995070.

• registerDataFile schedules a check only when the data file has no future, and stores the future it gets back after schedule returns (lines 421 to 450).
• When the check finds a newer file, updatedFileAvailable cancels and clears the stored future (lines 271 to 275). checkForUpdate then calls registerDataFile again to schedule the next check, and fires the update complete event after that (lines 242 to 252).
• The test's configureTimerImmediateCallback(1, null) runs the first check on the executor after 0 milliseconds, so the check can run on another thread while registerDataFile is still on its way to storing the future.

Why it fails

The second schedule and the event are on the same thread, in that order, so when the test wakes on the event any second schedule has already happened. "But was 1 time" therefore means the second schedule did not happen at all, so waiting longer in the test does not help.

Reading the code, one order of the two threads gives exactly that.

1. The check's updatedFileAvailable finds no future yet, so it clears nothing.
2. registerDataFile on the test thread stores the first future.
3. The check's registerDataFile finds that future and schedules nothing.

This has not been reproduced on purpose. It fits the count above, and it needs the check to start before the future is stored, which a 0 millisecond delay allows. DataUpdateService_UpdateFromUrl_UpdateAvailable checks for two schedules after the same kind of update (line 601), so it can fail the same way.

Options

1. Fix the service so a check cannot run before its future is stored, for example by scheduling and storing under the data file's update lock, which the check takes before it reads the future. That changes production code that every on-premise engine uses.
2. Change the test helper to give the first check a short delay. That makes the failure less likely without removing it.

What remains

• A test that shows the order above failing, and then the fix.
• The production intervals are minutes or more, so a check starting before its future is stored is not expected there. That has not been checked against the startup and retry paths.