Skip to content

Commit

Permalink
[MOREL-207] Detect and fix flaky tests
Browse files Browse the repository at this point in the history
Revert the change of TypeMap.typeVars from HashMap to
LinkedHashMap; it is fine as a HashMap because it is never
iterated over.

Switch from assertEquals to assertThat.

Fixes #207

Co-authored-by: Julian Hyde <jhyde@apache.org>
Co-authored-by: Rette66 <wenxiuw2@illinois.edu>
  • Loading branch information
Rette66 and julianhyde committed Dec 4, 2023
1 parent 3e50146 commit e420ca1
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -775,7 +775,7 @@ private RelContext group(RelContext cx, Core.Group group) {
}

private static EvalEnv evalEnvOf(Environment env) {
final Map<String, Object> map = new HashMap<>();
final Map<String, Object> map = new LinkedHashMap<>();
env.forEachValue(map::put);
EMPTY_ENV.visit(map::putIfAbsent);
return EvalEnvs.copyOf(map);
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/net/hydromatic/morel/compile/TypeMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ public class TypeMap {
public final TypeSystem typeSystem;
private final Map<AstNode, Unifier.Term> nodeTypeTerms;
final Unifier.Substitution substitution;

/** Map from type variable name to type variable. The ordinal of the variable
* is the size of the map at the time it is registered.
*
* <p>This map is never iterated over, and therefore the deterministic
* iteration provided by LinkedHashMap is not necessary, and HashMap is
* sufficient. */
private final Map<String, TypeVar> typeVars = new HashMap<>();

TypeMap(TypeSystem typeSystem, Map<AstNode, Unifier.Term> nodeTypeTerms,
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/net/hydromatic/morel/MainTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2114,7 +2114,7 @@ private static List<Object> node(Object... args) {
final String ml = "from s in [\"abc\", \"\", \"d\"],\n"
+ " c in explode s\n"
+ " group s compute count = sum of 1";
ml(ml).assertEvalIter(equalsOrdered(list(3, "abc"), list(1, "d")));
ml(ml).assertEvalIter(equalsUnordered(list(3, "abc"), list(1, "d")));
}

@Test void testJoinLateral() {
Expand Down Expand Up @@ -2333,7 +2333,7 @@ private static List<Object> node(Object... args) {
+ " group j = i mod 2\n"
+ " compute sum of j")
.assertType("{j:int, sum:int} list")
.assertEval(is(list(list(1, 5), list(0, 3))));
.assertEvalIter(equalsUnordered(list(1, 5), list(0, 3)));

// "compute" must not be followed by other steps
ml("from i in [1, 2, 3] compute s = sum of i yield s + 2")
Expand Down
4 changes: 3 additions & 1 deletion src/test/java/net/hydromatic/morel/SatTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import net.hydromatic.morel.util.Sat.Term;
import net.hydromatic.morel.util.Sat.Variable;

import com.google.common.collect.ImmutableMap;
import org.junit.jupiter.api.Test;

import java.util.Map;
Expand Down Expand Up @@ -50,7 +51,8 @@ public class SatTest {

final Map<Variable, Boolean> solution = sat.solve(formula);
assertThat(solution, notNullValue());
assertThat(solution.toString(), is("{x=false, y=true}"));
assertThat(solution,
is(ImmutableMap.of(x, false, y, true)));
}

/** Tests true ("and" with zero arguments). */
Expand Down

0 comments on commit e420ca1

Please sign in to comment.