detect and fix flaky tests

[Rette66]

Description

• Fixed the flaky test testBuild testCompute inside the MainTest class.
• Fixed the flaky test dumpToStringTwice inside the SatTest class.
• Detected the flaky test test inside the ScriptTest class.
• Detected the flaky test testNestedSchema inside the CalciteTest class.

Root Cause

All test mentioned above has been reported as flaky when run with the NonDex tool.

These tests failed because the asserteval() method would invoke a compile process for a hard-coded code and the compile process will eventually invoke this method:

morel/src/main/java/net/hydromatic/morel/compile/Compiler.java

Line 710 in b3ce1d8

final Object o = code.eval(evalEnv);

Inside the eval execution, methods in evalEnvOf() will be invoked and a new object with the type TypeMap will be created. These methods and class utilize HashMap which do not guarantee elements' order. Therefore, unexpected results will be generated while testing.

morel/src/main/java/net/hydromatic/morel/compile/CalciteCompiler.java

Lines 777 to 778 in b3ce1d8

	private static EvalEnv evalEnvOf(Environment env) {
	final Map<String, Object> map = new HashMap<>();

morel/src/main/java/net/hydromatic/morel/compile/TypeMap.java

Lines 44 to 48 in b3ce1d8

	public class TypeMap {
	public final TypeSystem typeSystem;
	private final Map<AstNode, Unifier.Term> nodeTypeTerms;
	final Unifier.Substitution substitution;
	private final Map<String, TypeVar> typeVars = new HashMap<>();

To reproduce run:

mvn -pl <module_name> edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=<test_name>

Fix

Tests are fixed by changing HashMap to LinkedHashMap which can guarantee element order and change the expected result with the correct executing order. Besides, for testBuild testCompute in MainTest class I change the matcher to equalsUnordered to check only elements of a map instead of their orders. For dumpToStringTwice inside the SatTest I create an expected map and use assertEquals to compare the tested result and the expected result.

Key changed/added classes in this PR

• CalciteTest.testNestedSchema
• SatTest.testBuild
• ScriptTest.test
• MainTest.testCrossApplyGroup
• MainTest.testCompute

[julianhyde]

I don't think changing typeVars from HashMap to LinkedHashMap is necessary; it is never iterated over.

We don't allow * imports even for statics, but you weren't to know; I've added a checkstyle rule.

I added a couple of fixup commits as https://github.com/julianhyde/morel/tree/207-flaky and will squash/merge if you're OK with them.

[julianhyde]

Merged (with slight modifications, as noted above). Thanks for the PR!

[Rette66]

Merged (with slight modifications, as noted above). Thanks for the PR!

Thanks for reviewing my PR! It makes more sense with your modifications!