This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit d4276bcc19523285ed9385a52741d15e16315e78 Author: Akshay Dayal <[email protected]> AuthorDate: Mon Jun 26 02:30:26 2023 -0600 [CALCITE-5786] QuidemTest and DiffRepository are not compatible with Gradle incremental builds since they write to build/resources This commit solves the problem by writing to quidem and diffrepo directories rather than resources. QuidemTest for src/test/resources/foo.iq now writes to build/quidem/test/foo.iq rather than build/resources/test/surefire/foo.iq DiffRepository for src/test/resources/com/example/FooTest.xml now writes to build/diffrepo/test/com/example/FooTest_actual.xml rather than build/resources/com/example/Foo_actual.xml Close apache/calcite#3280 --- .../org/apache/calcite/test/RelOptRulesTest.java | 10 +++-- .../org/apache/calcite/test/DiffRepository.java | 20 ++++++---- .../java/org/apache/calcite/test/QuidemTest.java | 44 ++++++++++++++-------- 3 files changed, 47 insertions(+), 27 deletions(-) diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java index f61185dbe4..a1408f089a 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -177,9 +177,13 @@ import static org.junit.jupiter.api.Assertions.fail; * for details on the schema. * * <li>Run the test. It should fail. Inspect the output in - * {@code build/resources/.../RelOptRulesTest_actual.xml} - * (e.g., {@code diff src/test/resources/.../RelOptRulesTest.xml - * build/resources/.../RelOptRulesTest_actual.xml}). + * {@code build/diffrepo/test/org/apache/calcite/test/RelOptRulesTest_actual.xml}, + * e.g. + * + * <blockquote>{@code + * diff core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml + * build/diffrepo/test/org/apache/calcite/test/RelOptRulesTest_actual.xml + * }</blockquote> * * <li>Verify that the "planBefore" is the correct translation of your SQL, * and that it contains the pattern on which your rule is supposed to fire. diff --git a/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java b/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java index f1052ea804..160d42dd8c 100644 --- a/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java +++ b/testkit/src/main/java/org/apache/calcite/test/DiffRepository.java @@ -76,11 +76,11 @@ import static java.util.Objects.requireNonNull; * return DiffRepository.lookup(MyTest.class); * } * - * @Test public void testToUpper() { + * @Test void testToUpper() { * getDiffRepos().assertEquals("${result}", "${string}"); * } * - * @Test public void testToLower() { + * @Test void testToLower() { * getDiffRepos().assertEquals("Multi-line\nstring", "${string}"); * } * } @@ -110,16 +110,18 @@ import static java.util.Objects.requireNonNull; * </code></pre></blockquote> * * <p>If any of the test cases fails, a log file is generated, called - * <code>build/resources/test/com/acme/test/MyTest_actual.xml</code>, containing the actual - * output. + * {@code build/diffrepo/test/com/acme/test/MyTest_actual.xml}, + * containing the actual output. * * <p>The log * file is otherwise identical to the reference log, so once the log file has * been verified, it can simply be copied over to become the new reference * log: * - * <blockquote><code>cp build/resources/test/com/acme/test/MyTest_actual.xml - * src/test/resources/com/acme/test/MyTest.xml</code></blockquote> + * <blockquote>{@code + * cp build/diffrepo/test/com/acme/test/MyTest_actual.xml + * src/test/resources/com/acme/test/MyTest.xml + * }</blockquote> * * <p>If a resource or test case does not exist, <code>DiffRepository</code> * creates them in the log file. Because DiffRepository is so forgiving, it is @@ -128,7 +130,7 @@ import static java.util.Objects.requireNonNull; * <p>The {@link #lookup} method ensures that all test cases share the same * instance of the repository. This is important more than one test case fails. * The shared instance ensures that the generated - * <code>build/resources/test/com/acme/test/MyTest_actual.xml</code> + * {@code build/diffrepo/test/com/acme/test/MyTest_actual.xml} * file contains the actual for <em>both</em> test cases. */ public class DiffRepository { @@ -927,7 +929,9 @@ public class DiffRepository { DiffRepository toRepo() { final URL refFile = findFile(clazz, ".xml"); final String refFilePath = Sources.of(refFile).file().getAbsolutePath(); - final String logFilePath = refFilePath.replace(".xml", "_actual.xml"); + final String logFilePath = refFilePath + .replace("resources", "diffrepo") + .replace(".xml", "_actual.xml"); final File logFile = new File(logFilePath); assert !refFilePath.equals(logFile.getAbsolutePath()); return new DiffRepository(refFile, logFile, baseRepository, filter, diff --git a/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java b/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java index 6a8f01abde..60bcd15c9d 100644 --- a/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java +++ b/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java @@ -39,6 +39,7 @@ import com.google.common.io.PatternFilenameFilter; import net.hydromatic.quidem.CommandHandler; import net.hydromatic.quidem.Quidem; +import org.checkerframework.checker.nullness.qual.Nullable; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -61,6 +62,8 @@ import java.util.regex.Pattern; import static org.junit.jupiter.api.Assertions.fail; +import static java.util.Objects.requireNonNull; + /** * Test that runs every Quidem file as a test. */ @@ -69,7 +72,7 @@ public abstract class QuidemTest { private static final Pattern PATTERN = Pattern.compile("\\.iq$"); - private static Object getEnv(String varName) { + private static @Nullable Object getEnv(String varName) { switch (varName) { case "jdk18": return System.getProperty("java.version").startsWith("1.8"); @@ -99,7 +102,7 @@ public abstract class QuidemTest { } } - private Method findMethod(String path) { + private @Nullable Method findMethod(String path) { // E.g. path "sql/agg.iq" gives method "testSqlAgg" final String path1 = path.replace(File.separatorChar, '_'); final String path2 = PATTERN.matcher(path1).replaceAll(""); @@ -113,11 +116,11 @@ public abstract class QuidemTest { return m; } - @SuppressWarnings("BetaApi") + @SuppressWarnings({"BetaApi", "UnstableApiUsage"}) protected static Collection<String> data(String first) { // inUrl = "file:/home/fred/calcite/core/target/test-classes/sql/agg.iq" final URL inUrl = QuidemTest.class.getResource("/" + n2u(first)); - final File firstFile = Sources.of(inUrl).file(); + final File firstFile = Sources.of(requireNonNull(inUrl, "inUrl")).file(); final int commonPrefixLength = firstFile.getAbsolutePath().length() - first.length(); final File dir = firstFile.getParentFile(); final List<String> paths = new ArrayList<>(); @@ -137,11 +140,14 @@ public abstract class QuidemTest { inFile = f; outFile = new File(path + ".out"); } else { - // e.g. path = "sql/outer.iq" - // inUrl = "file:/home/fred/calcite/core/target/test-classes/sql/outer.iq" + // e.g. path = "sql/agg.iq" + // inUrl = "file:/home/fred/calcite/core/build/resources/test/sql/agg.iq" + // inFile = "/home/fred/calcite/core/build/resources/test/sql/agg.iq" + // outDir = "/home/fred/calcite/core/build/quidem/test/sql" + // outFile = "/home/fred/calcite/core/build/quidem/test/sql/agg.iq" final URL inUrl = QuidemTest.class.getResource("/" + n2u(path)); - inFile = Sources.of(inUrl).file(); - outFile = new File(inFile.getAbsoluteFile().getParent(), u2n("surefire/") + path); + inFile = Sources.of(requireNonNull(inUrl, "inUrl")).file(); + outFile = replaceDir(inFile, "resources", "quidem"); } Util.discard(outFile.getParentFile().mkdirs()); try (Reader reader = Util.reader(inFile); @@ -179,6 +185,18 @@ public abstract class QuidemTest { } } + /** Returns a file, replacing one directory with another. + * + * <p>For example, {@code replaceDir("/abc/str/astro.txt", "str", "xyz")} + * returns "{@code "/abc/xyz/astro.txt}". + * Note that the file name "astro.txt" does not become "axyzo.txt". + */ + private static File replaceDir(File file, String target, String replacement) { + return new File( + file.getAbsolutePath().replace(n2u('/' + target + '/'), + n2u('/' + replacement + '/'))); + } + /** Creates a command handler. */ protected CommandHandler createCommandHandler() { return Quidem.EMPTY_COMMAND_HANDLER; @@ -189,14 +207,8 @@ public abstract class QuidemTest { return new QuidemConnectionFactory(); } - /** Converts a path from Unix to native. On Windows, converts - * forward-slashes to back-slashes; on Linux, does nothing. */ - private static String u2n(String s) { - return File.separatorChar == '\\' - ? s.replace('/', '\\') - : s; - } - + /** Converts a path from native to Unix. On Windows, converts + * back-slashes to forward-slashes; on Linux, does nothing. */ private static String n2u(String s) { return File.separatorChar == '\\' ? s.replace('\\', '/')
