zabetak commented on code in PR #3145: URL: https://github.com/apache/calcite/pull/3145#discussion_r1296836281
########## plus/src/test/java/org/apache/calcite/slt/SqlLogicTests.java: ########## @@ -0,0 +1,337 @@ +/* + * 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 org.apache.calcite.util.trace.CalciteTrace; + +import com.google.common.collect.ImmutableSet; + +import net.hydromatic.sqllogictest.OptionsParser; +import net.hydromatic.sqllogictest.TestStatistics; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.DynamicTest; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.TestFactory; +import org.junit.jupiter.api.function.Executable; +import org.slf4j.Logger; + +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.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. + * + * <p>For each test file the number of failed tests is saved 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 this tool does not track of the actual + * errors that were encountered; we expect that, as bugs are fixed in Calcite, + * the number of errors will shrink, and a more precise accounting method will be used. + * + * <p>The tests will fail if any test script generates + * *more* errors than the number from the golden file. Review Comment: nit: `* more *` appears strange in the Javadoc reader. Use an appropriate tag or remove the asterisks. ########## plus/src/test/java/org/apache/calcite/slt/SqlLogicTests.java: ########## @@ -0,0 +1,337 @@ +/* + * 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 org.apache.calcite.util.trace.CalciteTrace; + +import com.google.common.collect.ImmutableSet; + +import net.hydromatic.sqllogictest.OptionsParser; +import net.hydromatic.sqllogictest.TestStatistics; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.DynamicTest; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.TestFactory; +import org.junit.jupiter.api.function.Executable; +import org.slf4j.Logger; + +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.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. + * + * <p>For each test file the number of failed tests is saved 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 this tool does not track of the actual + * errors that were encountered; we expect that, as bugs are fixed in Calcite, + * the number of errors will shrink, and a more precise accounting method will be used. + * + * <p>The tests will fail if any test script generates + * *more* errors than the number from the golden file. + */ +public class SqlLogicTests { + private static final Logger LOGGER = + CalciteTrace.getTestTracer(SqlLogicTests.class); + + /** + * Short summary of the results of a test execution. + */ + public static class TestSummary { + /** + * File containing tests. + */ + final String file; + /** + * Number of tests that have failed. + */ + final int failed; + + TestSummary(String file, int failed) { + this.file = file; + this.failed = failed; + } + + /** + * Parses a TestSummary from a string. + * The inverse of 'toString'. + * + * @return The parsed TestSummary or null on failure. + */ + public static TestSummary parse(String line) { + String[] parts = line.split(":"); + if (parts.length != 2) { + return null; + } + try { + int failed = Integer.parseInt(parts[1]); + return new TestSummary(parts[0], failed); + } catch (NumberFormatException ex) { + return null; + } + } + + @Override public String toString() { + return this.file + ":" + this.failed; + } + + /** + * Check if the 'other' TestSummaries indicate a regressions Review Comment: nit: Usually we don't enclose parameters in quotes ('other'). I haven't seen this pattern before. -- 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]
