Skip to content

Fix nondeterministic iteration order in Fix Nondeterministic Iteration Order in CommentNotificationEventListenerTest - #3

Open
LucaDai wants to merge 1 commit into
mainfrom
FixNondeterministicIterationOrderCommentNotificationEventListenerTest
Open

Fix nondeterministic iteration order in Fix Nondeterministic Iteration Order in CommentNotificationEventListenerTest#3
LucaDai wants to merge 1 commit into
mainfrom
FixNondeterministicIterationOrderCommentNotificationEventListenerTest

Conversation

@LucaDai

@LucaDai LucaDai commented Sep 30, 2025

Copy link
Copy Markdown
Owner

Fix nondeterministic iteration order in CommentNotificationEventListenerTest

Summary

This PR fixes a nondeterministic failure in CommentNotificationEventListenerTest.
The nonderterministic can reporduce by running command:

mvn -pl jbpm-case-mgmt/jbpm-case-mgmt-impl \                                                 
  edu.illinois:nondex-maven-plugin:1.1.2:nondex \
  -Dtest=org.jbpm.casemgmt.impl.event.CommentNotificationEventListenerTest#testNotificationOnCommentAddedWithRawBody

The failure was caused by the use of a HashMap inside CommentNotificationEventListener when building notification parameters. Since HashMap does not guarantee iteration order, the generated user list sometimes appeared as [mary, john] and sometimes [john, mary]. The strict equality assertion in the test was sensitive to this order, leading to inconsistent results.

Relevant code in CommentNotificationEventListener:

public void afterCaseCommentAdded(CaseCommentEvent event) {
    buildAndPublishNotification(event);
}

private void buildAndPublishNotification(CaseCommentEvent event) {
    Map<String, Object> parameters = buildParams(event, commentContent);
    ...
}

protected Map<String, Object> buildParams(CaseCommentEvent event, StringBuilder commentContent) {
    Map<String, Object> parameters = new HashMap<>();
    ...
    return parameters;
}

While HashMap iteration order may appear stable in many runs, it is not guaranteed and can vary across JDK versions, platforms, or data sizes.

Error Message from Nondex:

[ERROR] Failures: 
[ERROR]   CommentNotificationEventListenerTest.testNotificationOnCommentAddedWithRawBody:184 expected:<...ment to [[UserImpl:'[mary'], [UserImpl:'john]']] with body simple...> but was:<...ment to [[UserImpl:'[john'], [UserImpl:'mary]']] with body simple...>
[INFO] 
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

Fix

Replaced the strict isEqualTo assertion with a regex matches assertion that accepts both possible orderings while still validating correctness:

String expectedNotification = "Publishing notification from cases@jbpm\\.org, with subject You have been mentioned in case \\(CASE-00001\\) comment to \\[\\[UserImpl:'(mary|john)'], \\[UserImpl:'(john|mary)']] with body simple comment for john and mary";

List<String> published = publisher.get();
assertThat(published).hasSize(1);
assertThat(published.get(0)).matches(expectedNotification);

This ensures the test is resilient to nondeterministic iteration order but continues to check that both "john" and "mary" are present in the notification.

Verification

  • Reproduced the failure using NonDex, a tool from the University of Illinois designed to detect ID tests (Iteration-Dependent tests).
  • NonDex perturbs the iteration order of Java standard library classes (e.g., HashMap, HashSet, getDeclaredFields) so that nondeterministic behaviors hidden in normal runs can be exposed.
  • Under the original strict equality check, NonDex triggered the failure when the HashMap iteration order produced [john, mary] instead of [mary, john].
  • After applying the regex-based fix, re-ran the test with NonDex (-DnondexRuns=20) under randomized orders, and all runs passed.
  • Confirmed that both possible user orderings are handled correctly by the regex.

Why this matters

Nondeterministic behavior may not surface in every run, but it introduces long-term risks for reliability and maintainability:

  • Different environments: Different JDK versions or JVM implementations may alter HashMap iteration behavior.
  • Scale effects: With more data or rehashing, iteration order can differ.
  • CI/CD reliability: Tests may pass locally but fail in automated pipelines.
  • Parallel execution: Concurrency may exacerbate ordering issues.

By making the test resilient to iteration order, we ensure stable, reproducible test results across environments and future JDK versions.

JIRA: N/A (class project / no ticket)

Referenced Pull Requests: N/A

@LucaDai LucaDai changed the title Fix nondeterministic iteration order in CommentNotificationEventListe… Fix nondeterministic iteration order in Fix Nondeterministic Iteration Order in CommentNotificationEventListenerTest Sep 30, 2025
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