Skip to content

Wait for the watcher's update in the watcher test, not the polling timer's check - #95

Merged
jwrosewell merged 2 commits into
rcfrom
fix/watcher-test-waits-for-update
Nov 10, 2026
Merged

jwrosewell merged 2 commits into
rcfrom
fix/watcher-test-waits-for-update

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Problem

DataUpdateService_UpdateFromWatcher in pipeline/pipeline.engines/src/test/java/fiftyone/pipeline/engines/services/DataUpdateServiceTests.java fails now and then on rc with "Wanted but not invoked" on refreshData. It failed in the full builds of two drafts that do not touch this test, Ubuntu Java 11 in run 42709779435 and Ubuntu Java 17 in run 42773815977. The failing test took 1.008 and 1.005 seconds.

The test enables the file watcher with a polling interval of one second, and waits for the first onUpdateComplete event. Two things complete within that second.

  • The polling timer's own check, checkForUpdate(dataFile, false), finds no update, because the file system watcher is enabled and there is no update URL, and still fires onUpdateComplete with AUTO_UPDATE_IN_PROGRESS.
  • The watcher's poll, dataFileUpdated, refreshes the engine through updatedFileAvailable and fires onUpdateComplete with AUTO_UPDATE_SUCCESS.

When the polling timer's completion comes first, it releases the test's flag before the watcher has refreshed the engine, and verify(engine, times(1)).refreshData(...) fails.

Change

Two commits, test code only. 4c9936a releases the flag only on a completion whose status is AUTO_UPDATE_SUCCESS, which only the watcher's update gives, with a comment saying why. 4eb2d3a adds the static import of AUTO_UPDATE_SUCCESS, which the test file on rc does not have. No main code changes.

Other drafts

This is the same change as d99c9b7 on #92, which drops it when it next takes rc. It merges before the other Java drafts so that their builds stop failing on this test.

No version directive is carried.

Full build

The full build and test matrix passed every job other than macOS on head 4eb2d3a in run 42783494993 on 10 November 2026, 11 jobs succeeded, run as a dry run so nothing merged. macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

…mer's check

With a one second polling interval the timer's check can complete first, finding no update because the watcher is enabled, and release the test before the watcher has refreshed the engine. The failed run took 1.008 seconds. The flag is now released only by a completion whose status is AUTO_UPDATE_SUCCESS, which only the watcher's update gives.
The watcher test now compares a completion's status with AUTO_UPDATE_SUCCESS, which rc's test file did not import.
@jwrosewell
jwrosewell marked this pull request as ready for review November 10, 2026 09:48
@jwrosewell
jwrosewell merged commit 30b38b9 into rc Nov 10, 2026
2 checks passed
@jwrosewell
jwrosewell deleted the fix/watcher-test-waits-for-update branch November 10, 2026 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant