Skip to content

Conversation

@Leo-Fish
Copy link

Hi! I ran NonDex on the test suite and found several tests with nondeterministic behavior. They're passing in normal CI but have dependencies on environment factors like hashmap iteration order and JVM version, which could potentially cause issues in different environments.

Nondeterministic tests identified:

(will keep adding to the list if I found more)

  • org.mitre.synthea.export.flexporter.CustomFHIRPathResourceGeneratorR4Test.testArray1
  • org.mitre.synthea.world.agents.PersonTest.testPersonRecreationSerialDifferentGenerator
  • org.mitre.synthea.world.agents.PersonTest.testPersonFhirSTU3Recreation
  • org.mitre.synthea.world.agents.PersonTest.testPersonRecreationParallel
  • org.mitre.synthea.world.agents.PersonTest.testPersonFhirDSTU2Recreation
  • org.mitre.synthea.world.agents.PersonTest.testPersonFhirR4Recreation
  • org.mitre.synthea.world.agents.PersonTest.testPersonCcdaRecreation

I've already identified fixes for some of these and will submit the rest incrementally. Happy to provide more details on any of these if helpful!

* Add sorting to ensure HashMap iteration order is deterministic
@Leo-Fish
Copy link
Author

Fixed: CustomFHIRPathResourceGeneratorR4Test.testArray1

Cause of nondeterminism: HashMap's non-deterministic iteration order in the sortedPaths() method caused inconsistent test results.

Proposed fix: Added explicit sorting with Collections.sort() to all three path categories (whereEquals, whereUnequals, withoutWhere) before combining them. This ensures deterministic ordering regardless of HashMap iteration order.

Note: Production code (Actions.java:422, createFhirPathMapping) already uses LinkedHashMap to address this issue in commit: 47fc1a9, but test code not being updated. I could apply the same LinkedHashMap approach here, but explicit sorting is more robust and prevents this in the future development. Happy to change to LinkedHashMap if you'd prefer to keep the approaches consistent :)

Changes:

  • Added Collections.sort() calls for all three path categories in sortedPaths()

Commit: c3da5c6

@Leo-Fish
Copy link
Author

Fixed: PayerTest.receiveMedicaidPregnancyEligible

Cause of nondeterminism: HashMap's non-deterministic iteration order in CSVEligibility constructor. While SimpleCSV.parseLineByLine() correctly returns LinkedHashMap, removeBlankMapStringValues() converted it back to HashMap, destroying insertion order and making the process non-deterministic.

Proposed fix: Modified removeBlankMapStringValues() to preserve LinkedHashMap type by specifying LinkedHashMap::new as the map supplier in Collectors.toMap(). This maintains the original CSV column order throughout the eligibility processing pipeline.

Changes:

  • Updated PlanEligibilityFinder.removeBlankMapStringValues() to use LinkedHashMap::new collector

Commit: c3da5c6

@codecov
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77%. Comparing base (0da3b77) to head (c3da5c6).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##             master   #1627   +/-   ##
========================================
- Coverage        77%     77%   -1%     
+ Complexity     4066    4062    -4     
========================================
  Files           180     180           
  Lines         25333   25340    +7     
  Branches       3615    3615           
========================================
- Hits          19678   19663   -15     
- Misses         4522    4545   +23     
+ Partials       1133    1132    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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