You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2023/06/29 00:17:37 UTC
[calcite] 01/02: [CALCITE-5786] QuidemTest and DiffRepository are not compatible with Gradle incremental builds since they write to build/resources
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 <ad...@gradle.com>
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('\\', '/')