zabetak commented on code in PR #3145:
URL: https://github.com/apache/calcite/pull/3145#discussion_r1265184045
##########
plus/src/test/java/org/apache/calcite/slt/README.md:
##########
@@ -0,0 +1,130 @@
+<!--
+{% comment %}
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to you under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+{% endcomment %}
+-->
+
+# Testing Calcite using SQL Logic Tests
+
+[SQL Logic Tests](https://www.sqlite.org/sqllogictest/doc/trunk/about.wiki) is
public domain suite of several million SQL tests
+created as part of the sqlite engine.
+The project
[hydromatic/sql-logic-test](https://github.com/hydromatic/sql-logic-test/)
+has packaged these tests into a Java framework which makes it easy to
+run them against a JDBC provider.
+
+## How does it work
+
+In this project we test Calcite as a JDBC provider, coupled with
[HSQLDB](http://hsqldb.org/) for storage.
+Another storage layer, such as [Postgres](https://www.postgresql.org), could
be substituted for HSQLDB.
+(Please note that the hydromatic/sql-logic-test project is configured
+to use the Postgres dialect version of the queries; for other dialects
+you may need to tweak the source code.)
+
+The test suite consists of 622 SQL scripts. Each script contains SQL
+statements and queries. The statements are executed using HSQLDB, while
+the queries are executed using Calcite.
+
+A small number of tests fail because they use statements currently not
supported by
+HSQLDB (these are listed in the `SqlLogicTestsForCalciteTests.unsupported`
set).
+
+For each test file we have summarized the number of passed and failed tests in
a "golden" file.
+These results are checked in as part of the `sltttestfailures.txt` resource
file.
+Currently, there are quite a few errors, so we do not keep tab of the actual
Review Comment:
's/tab/track/'
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
Review Comment:
If we don't care about the number of passed tests then why keep it and carry
it around ?
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
+ }
+ });
+ }
+ }
+
+ /**
+ * Check if 'other' summaries have regressions compared to `this`.
+ *
+ * @return 'true' if other contains regressions.
+ * @param other Test results to compare with.
+ * 'other' can contain only a subset of the tests.
+ */
+ boolean regression(AllTestSummaries other) {
+ boolean regression = false;
+ for (TestSummary summary: other.testResults.values()) {
+ TestSummary original = this.testResults.get(summary.file);
+ if (original == null) {
+ System.err.println("No historical data for test " + summary.file);
+ continue;
+ }
+ if (original.regression(summary)) {
+ System.err.println("Regression: " + original.file
+ + " had " + original.failed + " failures, now has " +
summary.failed);
+ regression = true;
+ }
+ }
+ return regression;
+ }
+
+ @Override public String toString() {
+ List<TestSummary> results = new ArrayList<>(this.testResults.values());
+ results.sort(Comparator.comparing(left -> left.file));
+ StringBuilder result = new StringBuilder();
+ for (TestSummary summary: results) {
+ result.append(summary.toString());
+ result.append(System.lineSeparator());
+ }
+ return result.toString();
+ }
+
+ /**
+ * Write the test results to the specified file.
+ */
+ public void writeToFile(File file) throws IOException {
+ try (BufferedWriter writer =
+ new BufferedWriter(
+ new OutputStreamWriter(
+ Files.newOutputStream(file.toPath()),
StandardCharsets.UTF_8))) {
+ writer.write(this.toString());
+ }
+ }
+
+ /**
+ * True if there is an entry for the specified test file.
+ *
+ * @param test Test file name.
+ */
+ public boolean contains(String test) {
+ return this.testResults.containsKey(test);
+ }
+ }
+
+ /**
+ * Summaries produced for the current run.
+ * Must be static since it is written by the `findRegressions`
+ * static method.
+ */
+ static AllTestSummaries testSummaries = new AllTestSummaries();
+ /**
+ * Summaries checked-in as resources that we compare against.
+ */
+ static AllTestSummaries goldenTestSummaries = new AllTestSummaries();
+
+ static final String GOLDENFILE = "/slttestfailures.txt";
+
+ private static TestStatistics launchSqlLogicTest(String... args) throws
IOException {
+ OptionsParser options = new OptionsParser(false, System.out, System.err);
+ CalciteExecutor.register(options);
+ return net.hydromatic.sqllogictest.Main.execute(options, args);
+ }
+
+ TestSummary shortSummary(String file, TestStatistics statistics) {
+ return new TestSummary(file, statistics.getPassedTestCount(),
statistics.getFailedTestCount());
+ }
+
+ // The following tests currently timeout during execution.
+ // Technically these are Calcite bugs.
+ Set<String> timeout = new HashSet<String>() {
+ {
+ add("test/select5.test");
+ add("test/random/groupby/slt_good_10.test");
+ }
+ };
+
+ // The following tests contain SQL statements that are not supported by
HSQLDB
Review Comment:
Idem with above.
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
+ }
+ });
+ }
+ }
+
+ /**
+ * Check if 'other' summaries have regressions compared to `this`.
+ *
+ * @return 'true' if other contains regressions.
+ * @param other Test results to compare with.
+ * 'other' can contain only a subset of the tests.
+ */
+ boolean regression(AllTestSummaries other) {
+ boolean regression = false;
+ for (TestSummary summary: other.testResults.values()) {
+ TestSummary original = this.testResults.get(summary.file);
+ if (original == null) {
+ System.err.println("No historical data for test " + summary.file);
+ continue;
+ }
+ if (original.regression(summary)) {
+ System.err.println("Regression: " + original.file
+ + " had " + original.failed + " failures, now has " +
summary.failed);
+ regression = true;
+ }
+ }
+ return regression;
+ }
+
+ @Override public String toString() {
+ List<TestSummary> results = new ArrayList<>(this.testResults.values());
+ results.sort(Comparator.comparing(left -> left.file));
+ StringBuilder result = new StringBuilder();
+ for (TestSummary summary: results) {
+ result.append(summary.toString());
+ result.append(System.lineSeparator());
+ }
+ return result.toString();
+ }
+
+ /**
+ * Write the test results to the specified file.
+ */
+ public void writeToFile(File file) throws IOException {
+ try (BufferedWriter writer =
+ new BufferedWriter(
+ new OutputStreamWriter(
+ Files.newOutputStream(file.toPath()),
StandardCharsets.UTF_8))) {
+ writer.write(this.toString());
+ }
+ }
+
+ /**
+ * True if there is an entry for the specified test file.
+ *
+ * @param test Test file name.
+ */
+ public boolean contains(String test) {
+ return this.testResults.containsKey(test);
+ }
+ }
+
+ /**
+ * Summaries produced for the current run.
+ * Must be static since it is written by the `findRegressions`
+ * static method.
+ */
+ static AllTestSummaries testSummaries = new AllTestSummaries();
+ /**
+ * Summaries checked-in as resources that we compare against.
+ */
+ static AllTestSummaries goldenTestSummaries = new AllTestSummaries();
+
+ static final String GOLDENFILE = "/slttestfailures.txt";
+
+ private static TestStatistics launchSqlLogicTest(String... args) throws
IOException {
+ OptionsParser options = new OptionsParser(false, System.out, System.err);
+ CalciteExecutor.register(options);
+ return net.hydromatic.sqllogictest.Main.execute(options, args);
+ }
+
+ TestSummary shortSummary(String file, TestStatistics statistics) {
+ return new TestSummary(file, statistics.getPassedTestCount(),
statistics.getFailedTestCount());
+ }
+
+ // The following tests currently timeout during execution.
+ // Technically these are Calcite bugs.
+ Set<String> timeout = new HashSet<String>() {
+ {
+ add("test/select5.test");
+ add("test/random/groupby/slt_good_10.test");
+ }
+ };
+
+ // The following tests contain SQL statements that are not supported by
HSQLDB
+ Set<String> unsupported = new HashSet<String>() {
+ {
+ add("test/evidence/slt_lang_replace.test");
+ add("test/evidence/slt_lang_createtrigger.test");
+ add("test/evidence/slt_lang_droptrigger.test");
+ add("test/evidence/slt_lang_update.test");
+ add("test/evidence/slt_lang_reindex.test");
+ }
+ };
+
+ void runOneTestFile(String testFile) throws IOException {
+ if (timeout.contains(testFile)) {
+ return;
+ }
+ if (unsupported.contains(testFile)) {
+ return;
+ }
+
+ TestStatistics res = launchSqlLogicTest("-v", "-e", "calcite", testFile);
+ assertThat(res, notNullValue());
+ assertThat(res.getParseFailureCount(), is(0));
+ assertThat(res.getIgnoredTestCount(), is(0));
+ assertThat(res.getTestFileCount(), is(1));
+ res.printStatistics(System.err); // Print errors found
+ TestSummary summary = this.shortSummary(testFile, res);
+ testSummaries.add(summary);
+ }
+
+ @Test @Tag("slow")
+ public void runOneTestFile() throws IOException {
+ runOneTestFile("select1.test");
+ }
+
+ @TestFactory @Disabled("This takes very long, should be run manually")
+ List<DynamicTest> runAllTests() {
+ // Run in parallel each test file.
+ Set<String> tests = net.hydromatic.sqllogictest.Main.getTestList();
+ List<DynamicTest> result = new ArrayList<>();
+ for (String test: tests) {
+ Executable executable = new Executable() {
+ @Override public void execute() {
+ assertTimeoutPreemptively(Duration.ofMinutes(10), () ->
runOneTestFile(test));
+ }
+ };
+ DynamicTest dynamicTest = DynamicTest.dynamicTest(test, executable);
+ result.add(dynamicTest);
+ }
+ return result;
+ }
+
+ @BeforeAll
+ public static void readGoldenFile() throws IOException {
+ // Read the statistics of the previously-failing tests
+ try (InputStream stream =
SqlLogicTestsForCalciteTests.class.getResourceAsStream(GOLDENFILE)) {
+ goldenTestSummaries.read(stream);
+ }
+ }
+
+ @AfterAll
+ public static void findRegressions() throws IOException {
+ // Compare with failures produced by a previous execution
+
+ // Code used to create the golden file originally
+ // File file = new File(goldenFile);
+ // if (!file.exists()) {
+ // testSummaries.writeToFile(file);
+ // return;
+ // }
Review Comment:
In general we avoid leaving commented out code in the repo. It is not that
important to know how the first file was generated but it is important to
document somewhere the process/steps to update the golden file when that is
necessary (e.g., someone fixes a bug and wants to update the file).
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
Review Comment:
We are already in Calcite so `ForCalciteTests` is a bit redundant; let's
keet it super simple and just use `SqlLogicTests`.
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
+ }
+ });
+ }
+ }
+
+ /**
+ * Check if 'other' summaries have regressions compared to `this`.
+ *
+ * @return 'true' if other contains regressions.
+ * @param other Test results to compare with.
+ * 'other' can contain only a subset of the tests.
+ */
+ boolean regression(AllTestSummaries other) {
+ boolean regression = false;
+ for (TestSummary summary: other.testResults.values()) {
+ TestSummary original = this.testResults.get(summary.file);
+ if (original == null) {
+ System.err.println("No historical data for test " + summary.file);
+ continue;
+ }
+ if (original.regression(summary)) {
+ System.err.println("Regression: " + original.file
+ + " had " + original.failed + " failures, now has " +
summary.failed);
+ regression = true;
+ }
+ }
+ return regression;
+ }
+
+ @Override public String toString() {
+ List<TestSummary> results = new ArrayList<>(this.testResults.values());
+ results.sort(Comparator.comparing(left -> left.file));
+ StringBuilder result = new StringBuilder();
+ for (TestSummary summary: results) {
+ result.append(summary.toString());
+ result.append(System.lineSeparator());
+ }
+ return result.toString();
+ }
+
+ /**
+ * Write the test results to the specified file.
+ */
+ public void writeToFile(File file) throws IOException {
+ try (BufferedWriter writer =
+ new BufferedWriter(
+ new OutputStreamWriter(
+ Files.newOutputStream(file.toPath()),
StandardCharsets.UTF_8))) {
+ writer.write(this.toString());
+ }
+ }
+
+ /**
+ * True if there is an entry for the specified test file.
+ *
+ * @param test Test file name.
+ */
+ public boolean contains(String test) {
+ return this.testResults.containsKey(test);
+ }
+ }
+
+ /**
+ * Summaries produced for the current run.
+ * Must be static since it is written by the `findRegressions`
+ * static method.
+ */
+ static AllTestSummaries testSummaries = new AllTestSummaries();
+ /**
+ * Summaries checked-in as resources that we compare against.
+ */
+ static AllTestSummaries goldenTestSummaries = new AllTestSummaries();
+
+ static final String GOLDENFILE = "/slttestfailures.txt";
+
+ private static TestStatistics launchSqlLogicTest(String... args) throws
IOException {
+ OptionsParser options = new OptionsParser(false, System.out, System.err);
+ CalciteExecutor.register(options);
+ return net.hydromatic.sqllogictest.Main.execute(options, args);
+ }
+
+ TestSummary shortSummary(String file, TestStatistics statistics) {
+ return new TestSummary(file, statistics.getPassedTestCount(),
statistics.getFailedTestCount());
+ }
+
+ // The following tests currently timeout during execution.
+ // Technically these are Calcite bugs.
+ Set<String> timeout = new HashSet<String>() {
+ {
+ add("test/select5.test");
+ add("test/random/groupby/slt_good_10.test");
+ }
+ };
+
+ // The following tests contain SQL statements that are not supported by
HSQLDB
+ Set<String> unsupported = new HashSet<String>() {
+ {
+ add("test/evidence/slt_lang_replace.test");
+ add("test/evidence/slt_lang_createtrigger.test");
+ add("test/evidence/slt_lang_droptrigger.test");
+ add("test/evidence/slt_lang_update.test");
+ add("test/evidence/slt_lang_reindex.test");
+ }
+ };
+
+ void runOneTestFile(String testFile) throws IOException {
+ if (timeout.contains(testFile)) {
Review Comment:
Better use JUnit assumptions
(https://junit.org/junit5/docs/5.0.3/api/org/junit/jupiter/api/Assumptions.html)
so that we have a complete picture of what is run and what is skipped and when
the latter happens to know also the reason.
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
+ }
+ });
+ }
+ }
+
+ /**
+ * Check if 'other' summaries have regressions compared to `this`.
+ *
+ * @return 'true' if other contains regressions.
+ * @param other Test results to compare with.
+ * 'other' can contain only a subset of the tests.
+ */
+ boolean regression(AllTestSummaries other) {
+ boolean regression = false;
+ for (TestSummary summary: other.testResults.values()) {
+ TestSummary original = this.testResults.get(summary.file);
+ if (original == null) {
+ System.err.println("No historical data for test " + summary.file);
+ continue;
+ }
+ if (original.regression(summary)) {
+ System.err.println("Regression: " + original.file
+ + " had " + original.failed + " failures, now has " +
summary.failed);
+ regression = true;
+ }
+ }
+ return regression;
+ }
+
+ @Override public String toString() {
+ List<TestSummary> results = new ArrayList<>(this.testResults.values());
+ results.sort(Comparator.comparing(left -> left.file));
+ StringBuilder result = new StringBuilder();
+ for (TestSummary summary: results) {
+ result.append(summary.toString());
+ result.append(System.lineSeparator());
+ }
+ return result.toString();
+ }
+
+ /**
+ * Write the test results to the specified file.
+ */
+ public void writeToFile(File file) throws IOException {
+ try (BufferedWriter writer =
+ new BufferedWriter(
+ new OutputStreamWriter(
+ Files.newOutputStream(file.toPath()),
StandardCharsets.UTF_8))) {
+ writer.write(this.toString());
+ }
+ }
+
+ /**
+ * True if there is an entry for the specified test file.
+ *
+ * @param test Test file name.
+ */
+ public boolean contains(String test) {
+ return this.testResults.containsKey(test);
+ }
+ }
+
+ /**
+ * Summaries produced for the current run.
+ * Must be static since it is written by the `findRegressions`
+ * static method.
+ */
+ static AllTestSummaries testSummaries = new AllTestSummaries();
+ /**
+ * Summaries checked-in as resources that we compare against.
+ */
+ static AllTestSummaries goldenTestSummaries = new AllTestSummaries();
+
+ static final String GOLDENFILE = "/slttestfailures.txt";
+
+ private static TestStatistics launchSqlLogicTest(String... args) throws
IOException {
+ OptionsParser options = new OptionsParser(false, System.out, System.err);
+ CalciteExecutor.register(options);
+ return net.hydromatic.sqllogictest.Main.execute(options, args);
+ }
+
+ TestSummary shortSummary(String file, TestStatistics statistics) {
+ return new TestSummary(file, statistics.getPassedTestCount(),
statistics.getFailedTestCount());
+ }
+
+ // The following tests currently timeout during execution.
+ // Technically these are Calcite bugs.
+ Set<String> timeout = new HashSet<String>() {
+ {
+ add("test/select5.test");
+ add("test/random/groupby/slt_good_10.test");
+ }
+ };
+
+ // The following tests contain SQL statements that are not supported by
HSQLDB
+ Set<String> unsupported = new HashSet<String>() {
+ {
+ add("test/evidence/slt_lang_replace.test");
+ add("test/evidence/slt_lang_createtrigger.test");
+ add("test/evidence/slt_lang_droptrigger.test");
+ add("test/evidence/slt_lang_update.test");
+ add("test/evidence/slt_lang_reindex.test");
+ }
+ };
+
+ void runOneTestFile(String testFile) throws IOException {
+ if (timeout.contains(testFile)) {
+ return;
+ }
+ if (unsupported.contains(testFile)) {
+ return;
+ }
+
+ TestStatistics res = launchSqlLogicTest("-v", "-e", "calcite", testFile);
+ assertThat(res, notNullValue());
+ assertThat(res.getParseFailureCount(), is(0));
+ assertThat(res.getIgnoredTestCount(), is(0));
+ assertThat(res.getTestFileCount(), is(1));
+ res.printStatistics(System.err); // Print errors found
+ TestSummary summary = this.shortSummary(testFile, res);
+ testSummaries.add(summary);
+ }
+
+ @Test @Tag("slow")
+ public void runOneTestFile() throws IOException {
+ runOneTestFile("select1.test");
+ }
+
+ @TestFactory @Disabled("This takes very long, should be run manually")
+ List<DynamicTest> runAllTests() {
+ // Run in parallel each test file.
+ Set<String> tests = net.hydromatic.sqllogictest.Main.getTestList();
+ List<DynamicTest> result = new ArrayList<>();
+ for (String test: tests) {
+ Executable executable = new Executable() {
+ @Override public void execute() {
+ assertTimeoutPreemptively(Duration.ofMinutes(10), () ->
runOneTestFile(test));
+ }
+ };
+ DynamicTest dynamicTest = DynamicTest.dynamicTest(test, executable);
+ result.add(dynamicTest);
+ }
+ return result;
+ }
+
+ @BeforeAll
Review Comment:
We don't really need to use a junit annotation since we simply have a static
constant initializer that does not change. I don't think we really need a
separate method to be honest.
With a tiny bit of refactoring in we could drop this method and just use
have the one in AllTestSumaries and have something like below:
```
private static final AllTestSummaries GOLDEN_SUMMARIES = new
AllTestSummaries().read("/slttestfailures.txt");
private static final AllTestSummaries GOLDEN_SUMMARIES =
AllTestSummaries.read("/slttestfailures.txt");
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
+ }
+ });
+ }
+ }
+
+ /**
+ * Check if 'other' summaries have regressions compared to `this`.
+ *
+ * @return 'true' if other contains regressions.
+ * @param other Test results to compare with.
+ * 'other' can contain only a subset of the tests.
+ */
+ boolean regression(AllTestSummaries other) {
+ boolean regression = false;
+ for (TestSummary summary: other.testResults.values()) {
+ TestSummary original = this.testResults.get(summary.file);
+ if (original == null) {
+ System.err.println("No historical data for test " + summary.file);
+ continue;
+ }
+ if (original.regression(summary)) {
+ System.err.println("Regression: " + original.file
+ + " had " + original.failed + " failures, now has " +
summary.failed);
+ regression = true;
+ }
+ }
+ return regression;
+ }
+
+ @Override public String toString() {
+ List<TestSummary> results = new ArrayList<>(this.testResults.values());
+ results.sort(Comparator.comparing(left -> left.file));
+ StringBuilder result = new StringBuilder();
+ for (TestSummary summary: results) {
+ result.append(summary.toString());
+ result.append(System.lineSeparator());
+ }
+ return result.toString();
+ }
+
+ /**
+ * Write the test results to the specified file.
+ */
+ public void writeToFile(File file) throws IOException {
+ try (BufferedWriter writer =
+ new BufferedWriter(
+ new OutputStreamWriter(
+ Files.newOutputStream(file.toPath()),
StandardCharsets.UTF_8))) {
+ writer.write(this.toString());
+ }
+ }
+
+ /**
+ * True if there is an entry for the specified test file.
+ *
+ * @param test Test file name.
+ */
+ public boolean contains(String test) {
+ return this.testResults.containsKey(test);
+ }
+ }
+
+ /**
+ * Summaries produced for the current run.
+ * Must be static since it is written by the `findRegressions`
+ * static method.
+ */
+ static AllTestSummaries testSummaries = new AllTestSummaries();
+ /**
+ * Summaries checked-in as resources that we compare against.
+ */
+ static AllTestSummaries goldenTestSummaries = new AllTestSummaries();
+
+ static final String GOLDENFILE = "/slttestfailures.txt";
+
+ private static TestStatistics launchSqlLogicTest(String... args) throws
IOException {
+ OptionsParser options = new OptionsParser(false, System.out, System.err);
+ CalciteExecutor.register(options);
+ return net.hydromatic.sqllogictest.Main.execute(options, args);
+ }
+
+ TestSummary shortSummary(String file, TestStatistics statistics) {
+ return new TestSummary(file, statistics.getPassedTestCount(),
statistics.getFailedTestCount());
+ }
+
+ // The following tests currently timeout during execution.
+ // Technically these are Calcite bugs.
+ Set<String> timeout = new HashSet<String>() {
Review Comment:
Not worth sub-classing `HashSet` for this. Use `ImmutableSet.of` or
`ImmutableSet.builder`. Same in other places where this pattern is used.
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
Review Comment:
I don't know if regression is the only thing that we should care about. If a
commit fixes a bug and we have more tests passing than before then it would be
nice to capture that information as well.
In an optimistic scenario where we keep fixing bugs and the number of
`failed` tests keeps dropping we will never need to update the golden file
hindering the progress that is made.
I am in favor of making the test fail (and requiring update the golden file
to pass) whenever there is a change in the passed/failed tests, which is inline
with what happens in general in the Calcite tests.
##########
plus/src/test/java/org/apache/calcite/slt/README.md:
##########
@@ -0,0 +1,130 @@
+<!--
+{% comment %}
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to you under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+{% endcomment %}
+-->
+
+# Testing Calcite using SQL Logic Tests
+
+[SQL Logic Tests](https://www.sqlite.org/sqllogictest/doc/trunk/about.wiki) is
public domain suite of several million SQL tests
+created as part of the sqlite engine.
+The project
[hydromatic/sql-logic-test](https://github.com/hydromatic/sql-logic-test/)
+has packaged these tests into a Java framework which makes it easy to
+run them against a JDBC provider.
+
+## How does it work
Review Comment:
This and the following sections could be the actual documentation (javadoc)
of `SqlLogicTestsForCalciteTests`. Not sure what's the benefit of having a
separate README file.
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
+ }
+ });
+ }
+ }
+
+ /**
+ * Check if 'other' summaries have regressions compared to `this`.
+ *
+ * @return 'true' if other contains regressions.
+ * @param other Test results to compare with.
+ * 'other' can contain only a subset of the tests.
+ */
+ boolean regression(AllTestSummaries other) {
+ boolean regression = false;
+ for (TestSummary summary: other.testResults.values()) {
+ TestSummary original = this.testResults.get(summary.file);
+ if (original == null) {
+ System.err.println("No historical data for test " + summary.file);
Review Comment:
This is not fatal so it should rather be `LOG.warn`
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
Review Comment:
This is not a Java app so we should not use `System.err` or `System.out` but
use loggers instead so that we can tune via config where the output goes.
##########
plus/src/test/java/org/apache/calcite/slt/README.md:
##########
@@ -0,0 +1,130 @@
+<!--
+{% comment %}
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to you under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+{% endcomment %}
+-->
+
+# Testing Calcite using SQL Logic Tests
+
+[SQL Logic Tests](https://www.sqlite.org/sqllogictest/doc/trunk/about.wiki) is
public domain suite of several million SQL tests
+created as part of the sqlite engine.
+The project
[hydromatic/sql-logic-test](https://github.com/hydromatic/sql-logic-test/)
+has packaged these tests into a Java framework which makes it easy to
+run them against a JDBC provider.
Review Comment:
This part could be moved to javadoc of package-info.java or removed
altogether. People who need more information about the sql-logic-test project
can visit the respective repo.
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
+ }
+ });
+ }
+ }
+
+ /**
+ * Check if 'other' summaries have regressions compared to `this`.
+ *
+ * @return 'true' if other contains regressions.
+ * @param other Test results to compare with.
+ * 'other' can contain only a subset of the tests.
+ */
+ boolean regression(AllTestSummaries other) {
+ boolean regression = false;
+ for (TestSummary summary: other.testResults.values()) {
+ TestSummary original = this.testResults.get(summary.file);
+ if (original == null) {
+ System.err.println("No historical data for test " + summary.file);
+ continue;
+ }
+ if (original.regression(summary)) {
+ System.err.println("Regression: " + original.file
+ + " had " + original.failed + " failures, now has " +
summary.failed);
+ regression = true;
+ }
+ }
+ return regression;
+ }
+
+ @Override public String toString() {
+ List<TestSummary> results = new ArrayList<>(this.testResults.values());
+ results.sort(Comparator.comparing(left -> left.file));
+ StringBuilder result = new StringBuilder();
+ for (TestSummary summary: results) {
+ result.append(summary.toString());
+ result.append(System.lineSeparator());
+ }
+ return result.toString();
+ }
+
+ /**
+ * Write the test results to the specified file.
+ */
+ public void writeToFile(File file) throws IOException {
+ try (BufferedWriter writer =
+ new BufferedWriter(
+ new OutputStreamWriter(
+ Files.newOutputStream(file.toPath()),
StandardCharsets.UTF_8))) {
+ writer.write(this.toString());
+ }
+ }
+
+ /**
+ * True if there is an entry for the specified test file.
+ *
+ * @param test Test file name.
+ */
+ public boolean contains(String test) {
+ return this.testResults.containsKey(test);
+ }
+ }
+
+ /**
+ * Summaries produced for the current run.
+ * Must be static since it is written by the `findRegressions`
+ * static method.
+ */
+ static AllTestSummaries testSummaries = new AllTestSummaries();
+ /**
+ * Summaries checked-in as resources that we compare against.
+ */
+ static AllTestSummaries goldenTestSummaries = new AllTestSummaries();
+
+ static final String GOLDENFILE = "/slttestfailures.txt";
+
+ private static TestStatistics launchSqlLogicTest(String... args) throws
IOException {
+ OptionsParser options = new OptionsParser(false, System.out, System.err);
+ CalciteExecutor.register(options);
+ return net.hydromatic.sqllogictest.Main.execute(options, args);
+ }
+
+ TestSummary shortSummary(String file, TestStatistics statistics) {
+ return new TestSummary(file, statistics.getPassedTestCount(),
statistics.getFailedTestCount());
+ }
+
+ // The following tests currently timeout during execution.
+ // Technically these are Calcite bugs.
Review Comment:
Turn it into Javadoc comment and move description from README.md here. It is
easier to keep things up-to-date if they appear in a single place.
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
Review Comment:
nit: 's/Parse/Parses/'
See
https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html
Use 3rd person (descriptive) not 2nd person (prescriptive).
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
+ }
+ });
+ }
+ }
+
+ /**
+ * Check if 'other' summaries have regressions compared to `this`.
+ *
+ * @return 'true' if other contains regressions.
+ * @param other Test results to compare with.
+ * 'other' can contain only a subset of the tests.
+ */
+ boolean regression(AllTestSummaries other) {
+ boolean regression = false;
+ for (TestSummary summary: other.testResults.values()) {
+ TestSummary original = this.testResults.get(summary.file);
+ if (original == null) {
+ System.err.println("No historical data for test " + summary.file);
+ continue;
+ }
+ if (original.regression(summary)) {
+ System.err.println("Regression: " + original.file
+ + " had " + original.failed + " failures, now has " +
summary.failed);
+ regression = true;
+ }
+ }
+ return regression;
+ }
+
+ @Override public String toString() {
+ List<TestSummary> results = new ArrayList<>(this.testResults.values());
+ results.sort(Comparator.comparing(left -> left.file));
+ StringBuilder result = new StringBuilder();
+ for (TestSummary summary: results) {
+ result.append(summary.toString());
+ result.append(System.lineSeparator());
+ }
+ return result.toString();
+ }
+
+ /**
+ * Write the test results to the specified file.
+ */
+ public void writeToFile(File file) throws IOException {
+ try (BufferedWriter writer =
+ new BufferedWriter(
+ new OutputStreamWriter(
+ Files.newOutputStream(file.toPath()),
StandardCharsets.UTF_8))) {
+ writer.write(this.toString());
+ }
+ }
+
+ /**
+ * True if there is an entry for the specified test file.
+ *
+ * @param test Test file name.
+ */
+ public boolean contains(String test) {
+ return this.testResults.containsKey(test);
+ }
+ }
+
+ /**
+ * Summaries produced for the current run.
+ * Must be static since it is written by the `findRegressions`
+ * static method.
+ */
+ static AllTestSummaries testSummaries = new AllTestSummaries();
+ /**
+ * Summaries checked-in as resources that we compare against.
+ */
+ static AllTestSummaries goldenTestSummaries = new AllTestSummaries();
+
+ static final String GOLDENFILE = "/slttestfailures.txt";
+
+ private static TestStatistics launchSqlLogicTest(String... args) throws
IOException {
+ OptionsParser options = new OptionsParser(false, System.out, System.err);
+ CalciteExecutor.register(options);
+ return net.hydromatic.sqllogictest.Main.execute(options, args);
+ }
+
+ TestSummary shortSummary(String file, TestStatistics statistics) {
+ return new TestSummary(file, statistics.getPassedTestCount(),
statistics.getFailedTestCount());
+ }
+
+ // The following tests currently timeout during execution.
+ // Technically these are Calcite bugs.
+ Set<String> timeout = new HashSet<String>() {
+ {
+ add("test/select5.test");
+ add("test/random/groupby/slt_good_10.test");
+ }
+ };
+
+ // The following tests contain SQL statements that are not supported by
HSQLDB
+ Set<String> unsupported = new HashSet<String>() {
+ {
+ add("test/evidence/slt_lang_replace.test");
+ add("test/evidence/slt_lang_createtrigger.test");
+ add("test/evidence/slt_lang_droptrigger.test");
+ add("test/evidence/slt_lang_update.test");
+ add("test/evidence/slt_lang_reindex.test");
+ }
+ };
+
+ void runOneTestFile(String testFile) throws IOException {
+ if (timeout.contains(testFile)) {
+ return;
+ }
+ if (unsupported.contains(testFile)) {
+ return;
+ }
+
+ TestStatistics res = launchSqlLogicTest("-v", "-e", "calcite", testFile);
+ assertThat(res, notNullValue());
+ assertThat(res.getParseFailureCount(), is(0));
+ assertThat(res.getIgnoredTestCount(), is(0));
+ assertThat(res.getTestFileCount(), is(1));
+ res.printStatistics(System.err); // Print errors found
+ TestSummary summary = this.shortSummary(testFile, res);
+ testSummaries.add(summary);
+ }
+
+ @Test @Tag("slow")
+ public void runOneTestFile() throws IOException {
+ runOneTestFile("select1.test");
+ }
+
+ @TestFactory @Disabled("This takes very long, should be run manually")
+ List<DynamicTest> runAllTests() {
+ // Run in parallel each test file.
+ Set<String> tests = net.hydromatic.sqllogictest.Main.getTestList();
+ List<DynamicTest> result = new ArrayList<>();
+ for (String test: tests) {
+ Executable executable = new Executable() {
+ @Override public void execute() {
+ assertTimeoutPreemptively(Duration.ofMinutes(10), () ->
runOneTestFile(test));
+ }
+ };
+ DynamicTest dynamicTest = DynamicTest.dynamicTest(test, executable);
+ result.add(dynamicTest);
+ }
+ return result;
+ }
+
+ @BeforeAll
+ public static void readGoldenFile() throws IOException {
+ // Read the statistics of the previously-failing tests
+ try (InputStream stream =
SqlLogicTestsForCalciteTests.class.getResourceAsStream(GOLDENFILE)) {
+ goldenTestSummaries.read(stream);
+ }
+ }
+
+ @AfterAll
+ public static void findRegressions() throws IOException {
+ // Compare with failures produced by a previous execution
+
+ // Code used to create the golden file originally
+ // File file = new File(goldenFile);
+ // if (!file.exists()) {
+ // testSummaries.writeToFile(file);
+ // return;
+ // }
+ boolean regression = goldenTestSummaries.regression(testSummaries);
+ Assertions.assertFalse(regression, "Regression discovered");
Review Comment:
It is not very common to put assertions in `@AfterAll` methods. The
assertion should be part of each test and not only do this at the very end.
##########
plus/build.gradle.kts:
##########
@@ -25,9 +25,11 @@ dependencies {
implementation("com.teradata.tpcds:tpcds")
implementation("io.prestosql.tpch:tpch")
implementation("net.hydromatic:chinook-data-hsqldb")
+ implementation("net.hydromatic:sql-logic-test:0.2")
implementation("net.hydromatic:tpcds")
implementation("org.apache.calcite.avatica:avatica-server")
implementation("org.hsqldb:hsqldb")
+ implementation("org.reflections:reflections:0.10.2")
Review Comment:
This question about the declaration of reflections here is still open.
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
Review Comment:
For instance when tests run in CI we don't want to clutter the console with
error messages.
##########
plus/src/test/java/org/apache/calcite/slt/SqlLogicTestsForCalciteTests.java:
##########
@@ -0,0 +1,312 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.slt;
+
+import org.apache.calcite.slt.executors.CalciteExecutor;
+
+import net.hydromatic.sqllogictest.OptionsParser;
+import net.hydromatic.sqllogictest.TestStatistics;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.function.Executable;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * Tests using sql-logic-test suite.
+ */
+public class SqlLogicTestsForCalciteTests {
+ /**
+ * Short summary of the results of a test execution.
+ */
+ static class TestSummary {
+ /**
+ * File containing tests.
+ */
+ final String file;
+ /**
+ * Number of tests that have passed.
+ */
+ final int passed;
+ /**
+ * Number of tests that have failed.
+ */
+ final int failed;
+
+ TestSummary(String file, int passed, int failed) {
+ this.file = file;
+ this.passed = passed;
+ this.failed = failed;
+ }
+
+ /**
+ * Parse a TestSummary from a string.
+ * The inverse of 'toString'.
+ *
+ * @return The parsed TestSummary or null on failure.
+ */
+ static TestSummary parse(String line) {
+ String[] parts = line.split(":");
+ if (parts.length != 3) {
+ return null;
+ }
+ try {
+ int passed = Integer.parseInt(parts[1]);
+ int failed = Integer.parseInt(parts[2]);
+ return new TestSummary(parts[0], passed, failed);
+ } catch (NumberFormatException ex) {
+ return null;
+ }
+ }
+
+ @Override public String toString() {
+ return this.file + ":" + this.passed + ":" + this.failed;
+ }
+
+ /**
+ * Check if the 'other' TestSummaries are a regressions
+ * when compared to 'this'.
+ *
+ * @param other TestSummary to compare against.
+ * @return 'true' if 'other' is a regression from 'this'.
+ */
+ boolean regression(TestSummary other) {
+ return other.failed > this.failed;
+ }
+ }
+
+ /**
+ * Summary for all tests executed.
+ */
+ static class AllTestSummaries {
+ /**
+ * Map test summary name to test summary.
+ */
+ final Map<String, TestSummary> testResults;
+
+ AllTestSummaries() {
+ this.testResults = new HashMap<>();
+ }
+
+ void add(TestSummary summary) {
+ this.testResults.put(summary.file, summary);
+ }
+
+ void read(InputStream stream) throws IOException {
+ try (BufferedReader reader =
+ new BufferedReader(new InputStreamReader(stream,
StandardCharsets.UTF_8))) {
+ reader.lines().forEach(line -> {
+ TestSummary summary = TestSummary.parse(line);
+ if (summary != null) {
+ this.add(summary);
+ } else {
+ System.err.println("Could not parse line " + line);
+ }
+ });
+ }
+ }
+
+ /**
+ * Check if 'other' summaries have regressions compared to `this`.
+ *
+ * @return 'true' if other contains regressions.
+ * @param other Test results to compare with.
+ * 'other' can contain only a subset of the tests.
+ */
+ boolean regression(AllTestSummaries other) {
+ boolean regression = false;
+ for (TestSummary summary: other.testResults.values()) {
+ TestSummary original = this.testResults.get(summary.file);
+ if (original == null) {
+ System.err.println("No historical data for test " + summary.file);
+ continue;
+ }
+ if (original.regression(summary)) {
+ System.err.println("Regression: " + original.file
+ + " had " + original.failed + " failures, now has " +
summary.failed);
+ regression = true;
+ }
+ }
+ return regression;
+ }
+
+ @Override public String toString() {
+ List<TestSummary> results = new ArrayList<>(this.testResults.values());
+ results.sort(Comparator.comparing(left -> left.file));
+ StringBuilder result = new StringBuilder();
+ for (TestSummary summary: results) {
+ result.append(summary.toString());
+ result.append(System.lineSeparator());
+ }
+ return result.toString();
+ }
+
+ /**
+ * Write the test results to the specified file.
+ */
+ public void writeToFile(File file) throws IOException {
+ try (BufferedWriter writer =
+ new BufferedWriter(
+ new OutputStreamWriter(
+ Files.newOutputStream(file.toPath()),
StandardCharsets.UTF_8))) {
+ writer.write(this.toString());
+ }
+ }
+
+ /**
+ * True if there is an entry for the specified test file.
+ *
+ * @param test Test file name.
+ */
+ public boolean contains(String test) {
+ return this.testResults.containsKey(test);
+ }
+ }
+
+ /**
+ * Summaries produced for the current run.
+ * Must be static since it is written by the `findRegressions`
+ * static method.
+ */
+ static AllTestSummaries testSummaries = new AllTestSummaries();
+ /**
+ * Summaries checked-in as resources that we compare against.
+ */
+ static AllTestSummaries goldenTestSummaries = new AllTestSummaries();
+
+ static final String GOLDENFILE = "/slttestfailures.txt";
+
+ private static TestStatistics launchSqlLogicTest(String... args) throws
IOException {
+ OptionsParser options = new OptionsParser(false, System.out, System.err);
+ CalciteExecutor.register(options);
+ return net.hydromatic.sqllogictest.Main.execute(options, args);
+ }
+
+ TestSummary shortSummary(String file, TestStatistics statistics) {
+ return new TestSummary(file, statistics.getPassedTestCount(),
statistics.getFailedTestCount());
+ }
+
+ // The following tests currently timeout during execution.
+ // Technically these are Calcite bugs.
+ Set<String> timeout = new HashSet<String>() {
+ {
+ add("test/select5.test");
+ add("test/random/groupby/slt_good_10.test");
+ }
+ };
+
+ // The following tests contain SQL statements that are not supported by
HSQLDB
+ Set<String> unsupported = new HashSet<String>() {
+ {
+ add("test/evidence/slt_lang_replace.test");
+ add("test/evidence/slt_lang_createtrigger.test");
+ add("test/evidence/slt_lang_droptrigger.test");
+ add("test/evidence/slt_lang_update.test");
+ add("test/evidence/slt_lang_reindex.test");
+ }
+ };
+
+ void runOneTestFile(String testFile) throws IOException {
+ if (timeout.contains(testFile)) {
+ return;
+ }
+ if (unsupported.contains(testFile)) {
+ return;
+ }
+
+ TestStatistics res = launchSqlLogicTest("-v", "-e", "calcite", testFile);
+ assertThat(res, notNullValue());
+ assertThat(res.getParseFailureCount(), is(0));
+ assertThat(res.getIgnoredTestCount(), is(0));
+ assertThat(res.getTestFileCount(), is(1));
+ res.printStatistics(System.err); // Print errors found
+ TestSummary summary = this.shortSummary(testFile, res);
+ testSummaries.add(summary);
+ }
+
+ @Test @Tag("slow")
+ public void runOneTestFile() throws IOException {
+ runOneTestFile("select1.test");
+ }
+
+ @TestFactory @Disabled("This takes very long, should be run manually")
+ List<DynamicTest> runAllTests() {
Review Comment:
Since `runAllTests()` is `@Disabled` there is a risk that the code gets
broken if it is not run in every day basis.
I would like to suggest a slight refactoring to ensure that we maximize the
code that is tested daily.
```
@TestFactory @Tag("slow")
List<DynamicTest> testSlow() throws IOException {
return generateTests(ImmutableSet.of("select1.test"));
}
@TestFactory @Disabled("This takes very long, should be run manually")
List<DynamicTest> testAll() throws IOException {
return generateTests(net.hydromatic.sqllogictest.Main.getTestList());
}
private static List<DynamicTest> generateTests(Set<String> testFiles) {
// This is the code in runAllTests
}
```
--
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]