LucasEby opened a new pull request, #6465: URL: https://github.com/apache/incubator-kie-drools/pull/6465
This PR removes a source of nondeterminism that caused `AccumulateTest#testTwoAccumulatesOnPattern` to fail intermittently under [NonDex](https://github.com/TestingResearchIllinois/NonDex) due to non-determinism. The bug was in code generation, not the test itself. To locate the nondeterminism and ensure it was fixed after modifying the code, I ran **NonDex**. NonDex systematically detects incorrect tests that rely on non-deterministic behaviors in Java APIs—like assuming `HashMap`’s iteration order—by exploring all specification-allowed outcomes. Failures it exposes reliably indicate flawed assumptions that should be fixed. --- ### Root Cause The executable-model generator built the RHS in two places using iteration over unordered collections: - `D.on(varA, varB, …)` — argument order passed to the consequence - `BlockN<…>.execute(T1 a, T2 b, …)` — parameter order/types the lambda expects . Each list was derived by iterating unordered collections so their orders could diverge. More specifically: - `existingDecls` was built with `Collectors.toSet()` which relies on [HashSets](https://docs.oracle.com/javase/8/docs/api/java/util/HashSet.html) which do not guarantee order - `getGlobals().keySet()` comes from a method that, when merging maps, returns a [HashMap](https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html) which does not guarantee order. - Rule-unit variables were also gathered without preserving a defined order. - Even when JavaParser returned a `List` of names, the code immediately did `.collect(toSet())`, again discarding order. Because `D.on(...)` and the lambda parameters were computed from these unordered sources, the two sequences could differ run-to-run, causing mismatched parameter ordering (compile-time type/arity errors or runtime failures when parameters were swapped). --- ### Fix Made the ordering deterministic and shared: 1. Build one canonical ordered list of candidate identifiers: - LHS declarations in declaration order (already stable) - Globals sorted by name (map order not guaranteed) - Rule-unit vars sorted by name 2. Filter that list to only the identifiers actually used in the RHS, preserving order via a `LinkedHashSet`. 3. Use this same list to generate both `D.on(...)` and the lambda parameter list. --- ### Why was the test not changed? Altering the test to avoid or ignore `$acc2` would sidestep the bug and weaken coverage. The test’s purpose is to verify that the rule’s RHS consumes `$acc2` correctly. Therefore, the original, stronger assertion was kept. --- ### Validation - Ran NonDex on the module: failures were reproduced before the fix. - No nondeterministic failures after the fix. - Behavior is unchanged except for stabilized parameter ordering. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
