[#116] Deterministic changelog wait in SunDSChangeLogSyncStrategyTests#117
Merged
vharseko merged 2 commits intoJul 21, 2026
Conversation
…IdentityPlatform#116) The stabilization heuristic (two equal lastChangeNumber reads 2s apart) is blind to changelog records OpenDJ has not written yet, so on slow runners a cleanup delete could land after the "stable" verdict and leak into the next test's sync window (expected:<1> but was:<2>). cleanupBaseContext() and LdapModifyForTests.modify() now report how many changes they made, and the tests wait until lastChangeNumber reaches the value read beforehand plus that count (plus two equal consecutive reads as a guard), with a 60s timeout instead of hanging forever.
vharseko
added
bug
vharseko
changed the title
maximthomas
reviewed
Jul 21, 2026
| } finally { | ||
| ctx.close(); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Suggested change
| } else { | |
| throw new IllegalArgumentException("Unsupported changeType: " + changeType); | |
| } |
maximthomas
approved these changes
Jul 21, 2026
…tyPlatform#116) Apply review suggestion: performChange() now throws IllegalArgumentException for an unsupported changeType instead of silently doing nothing, so a bad LDIF fails immediately rather than inflating the expected change number and timing out in waitForChangeLogToReach(). Also pass the missing dn argument to the "Deleting context" log placeholder.
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
2 participants
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.
Fixes #116.
Problem
waitForChangeLogToStabilize()treated two equallastChangeNumberreads 2 s apart as proof the changelog had settled. OpenDJ writes the retro changelog asynchronously, so the heuristic is blind to a record that has not been written yet: on a slow runner (Windows CI) the delete performed bycleanupBaseContext()landed after the "stable" verdict, leaked into the next sync window, andtestSimplegot two deltas instead of one (expected:<1> but was:<2>). The production sync code is correct — both records genuinely follow the token; the bug is purely in the test's wait.Fix
Make the wait deterministic — wait for the change number the log is known to owe the caller, instead of betting on timing:
SunDSTestBase.cleanupBaseContext()returns the number of entries it deleted.LdapModifyForTests.modify()returns the number of LDIF changes it performed.waitForChangeLogToStabilize()is replaced withwaitForChangeLogToReach(conn, expectedChangeNumber):newConnection()waits for the pre-cleanuplastChangeNumberplus the deleted count, anddoTest()waits for the sync token plus the LDIF change count. This also closes the mirror-image flake where the test's own change could miss its sync window (expected:<1> but was:<0>).AssertionErrorafter 60 s instead of hanging.The suite also got faster: 100 ms polling replaces the mandatory 2 s sleep per wait.
Testing
mvn -pl OpenICF-ldap-connector -Dtest='SunDSChangeLogSyncStrategyTests,VlvIndexSearchStrategyTests' test— 9/9 pass in ~52 s.