Skip to content

Commit

Permalink
[CALCITE-5726] Canonize use of Hamcrest matchers in test code
Browse files Browse the repository at this point in the history
Use matchers for Object.toString(), Collection.size(), Map.size():
* assertThat(map.size(), is(n)) → assertThat(map, aMapWithSize(n));
* assertThat(list.size(), is(n)) → assertThat(map, IsCollectionWithSize.hasSize(n));
* assertThat(o.toString(), is(s)) → assertThat(o, hasToString(s));
* assertThat(o.toString(), equalTo(s)) → assertThat(o, hasToString(s));

If there are multiple equivalent methods in Hamcrest, use the
ones in CoreMatchers or Matchers:
* CoreMatchers.is rather than Is.is
* Matchers.hasToString rather than HasToString.hasToString
* Matchers.hasSize rather than IsCollectionWithSize.hasSize
* Matchers.aMapWithSize rather than IsMapWithSize.aMapWithSize

Require static import of methods in CoreMatchers and Matchers.
Autostyle now converts

  import org.hamcrest.CoreMatchers;
  import org.hamcrest.Matchers;

to

  import static org.hamcrest.CoreMatchers.anything;
  import static org.hamcrest.Matchers.allOf;

and hopefully the user will take the hint.
  • Loading branch information
julianhyde committed May 29, 2023
1 parent 379f41d commit 0b819df
Show file tree
Hide file tree
Showing 91 changed files with 1,285 additions and 1,178 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/
package org.apache.calcite.test;

import org.apache.calcite.sql.SqlDialect;
import org.apache.calcite.sql.dialect.MysqlSqlDialect;
import org.apache.calcite.sql.dialect.PostgresqlSqlDialect;
Expand All @@ -39,6 +38,7 @@

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasToString;

/**
* Tests the "Babel" SQL parser, that understands all dialects of SQL.
Expand Down Expand Up @@ -453,7 +453,7 @@ private void checkParseInfixCast(String sqlType) {
+ "from `my emp` /* comment with 'quoted string'? */ as e\n"
+ "where deptno < ?3\n"
+ "and DATEADD(day, ?4, hiredate) > ?5";
assertThat(hoisted.toString(), is(expected));
assertThat(hoisted, hasToString(expected));

// Custom string converts variables to '[N:TYPE:VALUE]'
final String expected2 = "select [0:DECIMAL:1] as x,\n"
Expand Down
2 changes: 1 addition & 1 deletion babel/src/test/java/org/apache/calcite/test/BabelTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
import java.util.Properties;
import java.util.function.UnaryOperator;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;

/**
* Unit tests for Babel framework.
Expand Down
31 changes: 31 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,37 @@ allprojects {
replaceRegex(">[CALCITE-...] link styles: 1", "<a(?:(?!CALCITE-)[^>])++CALCITE-\\d+[^>]++>\\s*+\\[?(CALCITE-\\d+)\\]?", "<a href=\"https://issues.apache.org/jira/browse/\$1\">[\$1]")
// If the link was crafted manually, ensure it has [CALCITE-...] in the link text
replaceRegex(">[CALCITE-...] link styles: 2", "<a(?:(?!CALCITE-)[^>])++(CALCITE-\\d+)[^>]++>\\s*+\\[?CALCITE-\\d+\\]?", "<a href=\"https://issues.apache.org/jira/browse/\$1\">[\$1]")
replace("hamcrest: allOf", "org.hamcrest.core.AllOf.allOf", "org.hamcrest.CoreMatchers.allOf")
replace("hamcrest: aMapWithSize", "org.hamcrest.collection.IsMapWithSize.aMapWithSize", "org.hamcrest.Matchers.aMapWithSize")
replace("hamcrest: anyOf", "org.hamcrest.core.AnyOf.anyOf", "org.hamcrest.CoreMatchers.anyOf")
replace("hamcrest: containsString", "org.hamcrest.core.StringContains.containsString", "org.hamcrest.CoreMatchers.containsString")
replace("hamcrest: CoreMatchers", "import org.hamcrest.CoreMatchers;", "import static org.hamcrest.CoreMatchers.anything;")
replace("hamcrest: emptyArray", "org.hamcrest.collection.IsArrayWithSize.emptyArray", "org.hamcrest.Matchers.emptyArray")
replace("hamcrest: endsWidth", "org.hamcrest.core.StringEndsWith.endsWith", "org.hamcrest.CoreMatchers.endsWith")
replace("hamcrest: equalTo", "org.hamcrest.core.IsEqual.equalTo", "org.hamcrest.CoreMatchers.equalTo")
replace("hamcrest: greaterThanOrEqualTo", "org.hamcrest.number.OrderingComparison.greaterThanOrEqualTo", "org.hamcrest.Matchers.greaterThanOrEqualTo")
replace("hamcrest: greaterThan", "org.hamcrest.number.OrderingComparison.greaterThan", "org.hamcrest.Matchers.greaterThan")
replace("hamcrest: hasItem", "org.hamcrest.core.IsIterableContaining.hasItem", "org.hamcrest.CoreMatchers.hasItem")
replace("hamcrest: hasItems", "org.hamcrest.core.IsIterableContaining.hasItems", "org.hamcrest.CoreMatchers.hasItems")
replace("hamcrest: hasSize", "org.hamcrest.collection.IsCollectionWithSize.hasSize", "org.hamcrest.Matchers.hasSize")
replace("hamcrest: hasToString", "org.hamcrest.object.HasToString.hasToString", "org.hamcrest.Matchers.hasToString")
replace("hamcrest: instanceOf", "org.hamcrest.core.IsInstanceOf.instanceOf", "org.hamcrest.CoreMatchers.instanceOf")
replace("hamcrest: instanceOf", "org.hamcrest.Matchers.instanceOf", "org.hamcrest.CoreMatchers.instanceOf")
replace("hamcrest: is", "org.hamcrest.core.Is.is", "org.hamcrest.CoreMatchers.is")
replace("hamcrest: is", "org.hamcrest.Matchers.is", "org.hamcrest.CoreMatchers.is")
replace("hamcrest: isA", "org.hamcrest.core.Is.isA", "org.hamcrest.CoreMatchers.isA")
replace("hamcrest: lessThanOrEqualTo", "org.hamcrest.number.OrderingComparison.lessThanOrEqualTo", "org.hamcrest.Matchers.lessThanOrEqualTo")
replace("hamcrest: lessThan", "org.hamcrest.number.OrderingComparison.lessThan", "org.hamcrest.Matchers.lessThan")
replace("hamcrest: Matchers", "import org.hamcrest.Matchers;", "import static org.hamcrest.Matchers.allOf;")
replace("hamcrest: notNullValue", "org.hamcrest.core.IsNull.notNullValue", "org.hamcrest.CoreMatchers.notNullValue")
replace("hamcrest: notNullValue", "org.hamcrest.Matchers.notNullValue", "org.hamcrest.CoreMatchers.notNullValue")
replace("hamcrest: not", "org.hamcrest.core.IsNot.not", "org.hamcrest.CoreMatchers.not")
replace("hamcrest: not", "org.hamcrest.Matchers.not", "org.hamcrest.CoreMatchers.not")
replace("hamcrest: nullValue", "org.hamcrest.core.IsNull.nullValue", "org.hamcrest.CoreMatchers.nullValue")
replace("hamcrest: nullValue", "org.hamcrest.Matchers.nullValue", "org.hamcrest.CoreMatchers.nullValue")
replace("hamcrest: sameInstance", "org.hamcrest.core.IsSame.sameInstance", "org.hamcrest.CoreMatchers.sameInstance")
replace("hamcrest: startsWith", "org.hamcrest.core.StringStartsWith.startsWith", "org.hamcrest.CoreMatchers.startsWith")
replaceRegex("hamcrest: size", "\\.size\\(\\), (is|equalTo)\\(", ", hasSize\\(")
custom("((() preventer", 1) { contents: String ->
ParenthesisBalancer.apply(contents)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import com.google.common.collect.ImmutableList;

import org.checkerframework.checker.nullness.qual.Nullable;
import org.hamcrest.CoreMatchers;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -202,10 +201,10 @@ protected static Connection getRemoteConnection() throws SQLException {
assertThat(metaData.getColumnName(2), is("TABLE_CATALOG"));
assertThat(resultSet.next(), is(true));
assertThat(resultSet.getString(1), equalTo("POST"));
assertThat(resultSet.getString(2), CoreMatchers.nullValue());
assertThat(resultSet.getString(2), nullValue());
assertThat(resultSet.next(), is(true));
assertThat(resultSet.getString(1), equalTo("foodmart"));
assertThat(resultSet.getString(2), CoreMatchers.nullValue());
assertThat(resultSet.getString(2), nullValue());
assertThat(resultSet.next(), is(true));
assertThat(resultSet.next(), is(true));
assertThat(resultSet.next(), is(false));
Expand All @@ -226,7 +225,7 @@ protected static Connection getRemoteConnection() throws SQLException {
break;
case GET_SYSTEM_FUNCTIONS:
assertThat(connection.getMetaData().getSystemFunctions(),
CoreMatchers.notNullValue());
notNullValue());
break;
case GET_TIME_DATE_FUNCTIONS:
assertThat(connection.getMetaData().getTimeDateFunctions(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/
package org.apache.calcite.materialize;

import org.apache.calcite.prepare.PlannerImpl;
import org.apache.calcite.rel.RelRoot;
import org.apache.calcite.schema.SchemaPlus;
Expand Down Expand Up @@ -59,6 +58,9 @@
import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.aMapWithSize;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.hasToString;

/**
* Unit tests for {@link LatticeSuggester}.
Expand Down Expand Up @@ -135,7 +137,7 @@ class LatticeSuggesterTest {
+ " [scott, EMP]], "
+ "edges: [Step([scott, EMP], [scott, DEPT], DEPTNO:DEPTNO),"
+ " Step([scott, EMP], [scott, EMP], MGR:EMPNO)])";
assertThat(t.s.space.g.toString(), is(expected));
assertThat(t.s.space.g, hasToString(expected));
}

@Test void testFoodmart() throws Exception {
Expand Down Expand Up @@ -175,7 +177,7 @@ class LatticeSuggesterTest {
+ " product_id:product_id), "
+ "Step([foodmart, sales_fact_1997], [foodmart, time_by_day],"
+ " time_id:time_id)])";
assertThat(t.s.space.g.toString(), is(expected));
assertThat(t.s.space.g, hasToString(expected));
}

@Test void testAggregateExpression() throws Exception {
Expand Down Expand Up @@ -382,17 +384,17 @@ private void checkFoodMartAll(boolean evolve) throws Exception {
+ "Step([foodmart, warehouse], [foodmart, store], stores_id:store_id), "
+ "Step([foodmart, warehouse], [foodmart, warehouse_class],"
+ " warehouse_class_id:warehouse_class_id)])";
assertThat(t.s.space.g.toString(), is(expected));
assertThat(t.s.space.g, hasToString(expected));
if (evolve) {
// compared to evolve=false, there are a few more nodes (137 vs 119),
// the same number of paths, and a lot fewer lattices (27 vs 388)
assertThat(t.s.space.nodeMap.size(), is(137));
assertThat(t.s.latticeMap.size(), is(27));
assertThat(t.s.space.pathMap.size(), is(46));
assertThat(t.s.space.nodeMap, aMapWithSize(137));
assertThat(t.s.latticeMap, aMapWithSize(27));
assertThat(t.s.space.pathMap, aMapWithSize(46));
} else {
assertThat(t.s.space.nodeMap.size(), is(119));
assertThat(t.s.latticeMap.size(), is(388));
assertThat(t.s.space.pathMap.size(), is(46));
assertThat(t.s.space.nodeMap, aMapWithSize(119));
assertThat(t.s.latticeMap, aMapWithSize(388));
assertThat(t.s.space.pathMap, aMapWithSize(46));
}
}

Expand Down Expand Up @@ -435,7 +437,7 @@ private void checkFoodMartAll(boolean evolve) throws Exception {
+ "from \"sales_fact_1997\"";
final String l0 = "sales_fact_1997:[COUNT()]";
t.addQuery(q0);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
assertThat(Iterables.getOnlyElement(t.s.latticeMap.keySet()),
is(l0));

Expand All @@ -446,7 +448,7 @@ private void checkFoodMartAll(boolean evolve) throws Exception {
final String l1 = "sales_fact_1997 (customer:customer_id)"
+ ":[COUNT(), SUM(sales_fact_1997.unit_sales)]";
t.addQuery(q1);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
assertThat(Iterables.getOnlyElement(t.s.latticeMap.keySet()),
is(l1));

Expand All @@ -459,7 +461,7 @@ private void checkFoodMartAll(boolean evolve) throws Exception {
+ ":[COUNT(), SUM(sales_fact_1997.unit_sales),"
+ " COUNT(DISTINCT time_by_day.the_day)]";
t.addQuery(q2);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
assertThat(Iterables.getOnlyElement(t.s.latticeMap.keySet()),
is(l2));

Expand All @@ -469,8 +471,8 @@ private void checkFoodMartAll(boolean evolve) throws Exception {
table.t.getQualifiedName())
.sorted(Comparator.comparing(Object::toString))
.collect(Util.toImmutableList());
assertThat(tableNames.toString(),
is("[[foodmart, customer],"
assertThat(tableNames,
hasToString("[[foodmart, customer],"
+ " [foodmart, product],"
+ " [foodmart, sales_fact_1997],"
+ " [foodmart, time_by_day]]"));
Expand All @@ -486,7 +488,7 @@ private void checkFoodMartAll(boolean evolve) throws Exception {
+ ":[COUNT(), SUM(sales_fact_1997.unit_sales),"
+ " MIN(product.product_id), COUNT(DISTINCT time_by_day.the_day)]";
t.addQuery(q3);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
assertThat(Iterables.getOnlyElement(t.s.latticeMap.keySet()),
is(l3));
}
Expand All @@ -502,15 +504,15 @@ private void checkFoodMartAll(boolean evolve) throws Exception {
+ "group by \"fname\", \"lname\"";
final String l0 = "customer:[COUNT(), AVG($f2)]";
t.addQuery(q0);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
assertThat(Iterables.getOnlyElement(t.s.latticeMap.keySet()),
is(l0));
final Lattice lattice = Iterables.getOnlyElement(t.s.latticeMap.values());
final List<Lattice.DerivedColumn> derivedColumns = lattice.columns.stream()
.filter(c -> c instanceof Lattice.DerivedColumn)
.map(c -> (Lattice.DerivedColumn) c)
.collect(Collectors.toList());
assertThat(derivedColumns.size(), is(2));
assertThat(derivedColumns, hasSize(2));
final List<String> tables = ImmutableList.of("customer");
checkDerivedColumn(lattice, tables, derivedColumns, 0, "$f2", true);
checkDerivedColumn(lattice, tables, derivedColumns, 1, "full_name", false);
Expand Down Expand Up @@ -545,7 +547,7 @@ private void checkFoodMartAll(boolean evolve) throws Exception {
// n14 = [_, dimension] -> not always measure
t.addQuery(q0);
t.addQuery(q1);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
final String l0 =
"customer:[COUNT(), SUM(n10), SUM(n11), SUM(n12), SUM(n13)]";
assertThat(Iterables.getOnlyElement(t.s.latticeMap.keySet()),
Expand All @@ -555,7 +557,7 @@ private void checkFoodMartAll(boolean evolve) throws Exception {
.filter(c -> c instanceof Lattice.DerivedColumn)
.map(c -> (Lattice.DerivedColumn) c)
.collect(Collectors.toList());
assertThat(derivedColumns.size(), is(5));
assertThat(derivedColumns, hasSize(5));
final List<String> tables = ImmutableList.of("customer");

checkDerivedColumn(lattice, tables, derivedColumns, 0, "n10", false);
Expand Down Expand Up @@ -586,15 +588,15 @@ private void checkDerivedColumn(Lattice lattice, List<String> tables,
final String l0 = "sales_fact_1997 (customer:customer_id)"
+ ":[COUNT(), AVG($f2)]";
t.addQuery(q0);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
assertThat(Iterables.getOnlyElement(t.s.latticeMap.keySet()),
is(l0));
final Lattice lattice = Iterables.getOnlyElement(t.s.latticeMap.values());
final List<Lattice.DerivedColumn> derivedColumns = lattice.columns.stream()
.filter(c -> c instanceof Lattice.DerivedColumn)
.map(c -> (Lattice.DerivedColumn) c)
.collect(Collectors.toList());
assertThat(derivedColumns.size(), is(2));
assertThat(derivedColumns, hasSize(2));
final List<String> tables = ImmutableList.of("customer");
assertThat(derivedColumns.get(0).tables, is(tables));
assertThat(derivedColumns.get(1).tables, is(tables));
Expand All @@ -619,7 +621,7 @@ private void checkDerivedColumn(Lattice lattice, List<String> tables,
+ "from \"customer\" join \"sales_fact_1997\" using (\"customer_id\")\n"
+ "group by \"fname\", \"lname\"";
t.addQuery(q0);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
}

/** Tests a number of features only available in BigQuery: back-ticks;
Expand All @@ -637,7 +639,7 @@ private void checkDerivedColumn(Lattice lattice, List<String> tables,
+ " `sales_fact_1997`"
+ "group by 1";
t.addQuery(q0);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
}

/** A tricky case involving a CTE (WITH), a join condition that references an
Expand Down Expand Up @@ -675,7 +677,7 @@ private void checkDerivedColumn(Lattice lattice, List<String> tables,
t.addQuery(q1);
t.addQuery(q4);
t.addQuery(q2);
assertThat(t.s.latticeMap.size(), is(3));
assertThat(t.s.latticeMap, aMapWithSize(3));
}

@Test void testDerivedColRef() throws Exception {
Expand All @@ -691,11 +693,11 @@ private void checkDerivedColumn(Lattice lattice, List<String> tables,
+ "left join \"sales_fact_1997\" as s\n"
+ "on c.\"customer_id\" + 1 = s.\"customer_id\" + 2";
t.addQuery(q0);
assertThat(t.s.latticeMap.size(), is(1));
assertThat(t.s.latticeMap, aMapWithSize(1));
assertThat(t.s.latticeMap.keySet().iterator().next(),
is("sales_fact_1997 (customer:+($2, 2)):[MIN(customer.fname)]"));
assertThat(t.s.space.g.toString(),
is("graph(vertices: [[foodmart, customer],"
assertThat(t.s.space.g,
hasToString("graph(vertices: [[foodmart, customer],"
+ " [foodmart, sales_fact_1997]], "
+ "edges: [Step([foodmart, sales_fact_1997],"
+ " [foodmart, customer], +($2, 2):+($0, 1))])"));
Expand Down Expand Up @@ -753,7 +755,7 @@ private void checkUnion(String setOp) throws Exception {

// Adding a query generates two lattices
final List<Lattice> latticeList = t.addQuery(q);
assertThat(latticeList.size(), is(2));
assertThat(latticeList, hasSize(2));

// But because of 'evolve' flag, the lattices are merged into a single
// lattice
Expand Down Expand Up @@ -851,7 +853,7 @@ List<Lattice> addQuery(String q) throws SqlParseException,
LatticeRootNode node(String q) throws SqlParseException,
ValidationException, RelConversionException {
final List<Lattice> list = addQuery(q);
assertThat(list.size(), is(1));
assertThat(list, hasSize(1));
return list.get(0).rootNode;
}

Expand Down
Loading

0 comments on commit 0b819df

Please sign in to comment.