-
Notifications
You must be signed in to change notification settings - Fork 812
Fix non-deterministic tests caused by iteration order #1627
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
* Add sorting to ensure HashMap iteration order is deterministic
Fixed: CustomFHIRPathResourceGeneratorR4Test.testArray1Cause 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:
Commit: c3da5c6 |
Fixed: PayerTest.receiveMedicaidPregnancyEligibleCause 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:
Commit: c3da5c6 |
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
…PersonTest - Use LinkedHashSet in ClinicalNoteExporter for deterministic clinical note ordering - Use ConcurrentSkipListMap for Person.chronicMedications - Use LinkedHashMap in HealthRecord and Demographics
Fixed: Remaining indeterministic PersonTest. All issues stemmed from HashMap/Set iteration order.Summary of Changes:Fix for CCDA/FHIR Recreation: Updated ClinicalNoteExporter to use LinkedHashSet instead of HashSet for active condition sets. |
Hi! I ran NonDex Tool created by the researchers of UIUC 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:
Reproduce
1 build the project
./gradlew build -x test2. Add the following to the plugins block in your build.gradle file:
plugins { id 'edu.illinois.nondex' version '2.1.7' }3.Run NonDex on the specific test:
./gradlew nondexTest --tests=org.mitre.synthea.export.flexporter.CustomFHIRPathResourceGeneratorR4Test.testArray1 --nondexRuns=50(You can replace the test method/class with the other affected tests listed above)
Happy to provide more details on any of these if helpful!