Skip to content

Stabilize flaky CleanupManagerTest.testCleanupHappens#262

Merged
vharseko merged 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:fix/cleanupmanager-flaky-happens
Jul 11, 2026
Merged

Stabilize flaky CleanupManagerTest.testCleanupHappens#262
vharseko merged 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:fix/cleanupmanager-flaky-happens

Conversation

@vharseko

Copy link
Copy Markdown
Member

Problem

CleanupManagerTest.testCleanupHappens is flaky. It offers 500 cleanup actions and then waits for them to run — but the wait loops only while the queue is non-empty:

cm().setPollInterval(100);
for (int i = 0; i < 10 && cm().getEnqueuedCount() > 0; i++) {
    Thread.sleep(1000);
}
assertEquals(500, _counter);

getEnqueuedCount() drops to 0 as soon as the background CLEANUP_MANAGER thread dequeues the batch — which happens before performAction runs. So on a slow/loaded runner the wait loop sees getEnqueuedCount() == 0 on the first check, exits without sleeping, and asserts while no action has been performed yet:

java.lang.AssertionError: expected:<500> but was:<0>
    at com.persistit.CleanupManagerTest.testCleanupHappens(...)

Observed on build-maven (ubuntu-latest, 17); passes elsewhere.

Fix

Wait on the manager's own performed/error counters (getPerformedCount() + getErrorCount()), which advance only after each action completes, until all 500 are processed — with a 30 s cap. This waits for the actions to actually run rather than merely leave the queue. All four existing assertions (_counter == 500, errorCount == 1, performedCount == 499, in-order) are unchanged.

Not caused by this PR's parent change

Unrelated to the PR it was blocking (#228, make Key.SDF thread-safe): that change is a SimpleDateFormat → ThreadLocal in Key, nothing to do with the cleanup manager. CleanupManagerTest was previously touched by #240 for a different race (out-of-order recording), not this queue-drain wait.

Verification

  • testCleanupHappens 6/6 green; full CleanupManagerTest class 3/3.
  • Root cause precisely identified (queue-count drops at dequeue, before performAction), so this is a deterministic fix.

The test offered 500 cleanup actions and waited only while
getEnqueuedCount() > 0. That counter drops to 0 as soon as the background
CLEANUP_MANAGER thread dequeues the batch -- before performAction runs -- so on
a slow runner the wait loop exited with no action yet performed and _counter
still 0 (expected:<500> but was:<0>, seen on ubuntu-latest JDK 17).

Wait on the manager's own performed/error counters (which advance only after
each action completes) until all 500 are processed, with a 30s cap.
@vharseko vharseko requested a review from maximthomas July 10, 2026 07:07
@vharseko vharseko added the tests Test code changes label Jul 10, 2026
The manager's performed/error counters are cumulative for the Persistit
instance, and startup maintenance may already have performed cleanup
actions before the test enqueues its own - in particular the
pruneTimelyResources() call added to Persistit.initialize() by OpenIdentityPlatform#266.
Waiting for and asserting absolute values could then exit early or fail;
take a baseline before enqueueing and compare deltas instead.
@vharseko vharseko merged commit 2a0e784 into OpenIdentityPlatform:master Jul 11, 2026
14 checks passed
@vharseko vharseko deleted the fix/cleanupmanager-flaky-happens branch July 11, 2026 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Test code changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants