Remove yaml-tests dependency on fdb-record-layer-core test code

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/ExcludeYamlTestConfig.java

@@ -26,7 +26,7 @@
 import java.lang.annotation.RetentionPolicy;
 
 /**
- * Mark specific {@link YamlTestConfig} as disabled for the current test in an {@link YamlTest} test class.
+ * Mark specific {@link YamlTestConfig} as disabled for the current test in an {@link YamlTestExtension} test class.
  * <p>
  *     Any config in {@link #value()} will be skipped for the annotated test.
  * </p>

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/MaintainYamlTestConfig.java

@@ -27,7 +27,7 @@
 
 /**
  * Mark specific {@link YamlTestConfig} that are designated test maintenance helper configs as enabled for the current
- * test in an {@link YamlTest} test class.
+ * test in an {@link YamlTestExtension} test class.
  * <p>
  *     Any config in {@link #value()} will be included for the annotated test. Any config not included will be skipped.
  * </p>

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlRunner.java

@@ -56,18 +56,18 @@ public final class YamlRunner {
     static final String TEST_NIGHTLY_REPETITION = "tests.yaml.iterations";
 
     @Nonnull
-    private final String resourcePath;
-
+    private final YamlConnectionFactory factory;
     @Nonnull
-    private final YamlExecutionContext executionContext;
+    private final YamlExecutionContext.ContextOptions additionalOptions;
 
-    public YamlRunner(@Nonnull String resourcePath, @Nonnull YamlConnectionFactory factory,
-                      @Nonnull final YamlExecutionContext.ContextOptions additionalOptions) throws RelationalException {
-        this.resourcePath = resourcePath;
-        this.executionContext = new YamlExecutionContext(resourcePath, factory, additionalOptions);
+    public YamlRunner(@Nonnull YamlConnectionFactory factory,
+                      @Nonnull final YamlExecutionContext.ContextOptions additionalOptions) {
+        this.factory = factory;
+        this.additionalOptions = additionalOptions;
     }
 
-    public void run() throws Exception {
+    public void runYamsql(@Nonnull String resourcePath) throws Exception {
+        YamlExecutionContext executionContext = new YamlExecutionContext(resourcePath, factory, additionalOptions);
         try {
             LoaderOptions loaderOptions = new LoaderOptions();
             loaderOptions.setAllowDuplicateKeys(true);
@@ -92,16 +92,16 @@ public void run() throws Exception {
                 block.execute();
             }
 
-            evaluateTestBlockResults(testBlocks);
-            replaceTestFileIfRequired();
-            replaceMetricsFileIfRequired();
+            evaluateTestBlockResults(testBlocks, resourcePath);
+            replaceTestFileIfRequired(executionContext);
+            replaceMetricsFileIfRequired(executionContext);
         } catch (RelationalException | IOException e) {
             logger.error("‼️ running test file '{}' was not successful", resourcePath, e);
             throw e;
         }
     }
 
-    private void evaluateTestBlockResults(List<TestBlock> testBlocks) {
+    private void evaluateTestBlockResults(List<TestBlock> testBlocks, final String resourcePath) {
         logger.info("");
         logger.info("");
         logger.info("--------------------------------------------------------------------------------------------------------------");
@@ -140,24 +140,24 @@ private static InputStream getInputStream(@Nonnull final String resourcePath) th
         return inputStream;
     }
 
-    private void replaceTestFileIfRequired() {
+    private void replaceTestFileIfRequired(final YamlExecutionContext executionContext) {
         if (executionContext.getEditedFileStream() == null || !executionContext.isDirty()) {
             return;
         }
         try {
-            try (var writer = new PrintWriter(new FileWriter(Path.of(System.getProperty("user.dir")).resolve(Path.of("src", "test", "resources", resourcePath)).toAbsolutePath().toString(), StandardCharsets.UTF_8))) {
+            try (var writer = new PrintWriter(new FileWriter(Path.of(System.getProperty("user.dir")).resolve(Path.of("src", "test", "resources", executionContext.resourcePath)).toAbsolutePath().toString(), StandardCharsets.UTF_8))) {
                 for (var line : executionContext.getEditedFileStream()) {
                     writer.println(line);
                 }
             }
-            logger.info("🟢 Source file {} replaced.", resourcePath);
+            logger.info("🟢 Source file {} replaced.", executionContext.resourcePath);
         } catch (IOException e) {
-            logger.error("⚠️ Source file {} could not be replaced with corrected file.", resourcePath);
+            logger.error("⚠️ Source file {} could not be replaced with corrected file.", executionContext.resourcePath);
             Assertions.fail(e);
         }
     }
 
-    private void replaceMetricsFileIfRequired() throws RelationalException {
+    private void replaceMetricsFileIfRequired(final YamlExecutionContext executionContext) throws RelationalException {
         if (!executionContext.isDirtyMetrics()) {
             return;
         }

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlTestConfigFilters.java

@@ -25,7 +25,7 @@
 import com.apple.foundationdb.relational.yamltests.configs.YamlTestConfig;
 
 /**
- * An enum of reasons to disable a {@link YamlTest} test for a given {@link YamlTestConfig}.
+ * An enum of reasons to disable a {@link YamlTestExtension} test for a given {@link YamlTestConfig}.
  * <p>
  *     Note: Part of the reason this exists as an enum, is that fields on annotations can only be certain types, and
  *     enum seemed like the cleanest.

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/YamlTestExtension.java

@@ -35,7 +35,6 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.extension.AfterAllCallback;
 import org.junit.jupiter.api.extension.BeforeAllCallback;
 import org.junit.jupiter.api.extension.Extension;
@@ -57,7 +56,20 @@
 import java.util.stream.Stream;
 
 /**
- * Extension that backs {@link YamlTest}.
+ * An extension for test classes that run {@code .yamsql} files.
+ * <p>
+ *     Test classes extended with this should have all their tests annotated with
+ *     {@link org.junit.jupiter.api.TestTemplate}, instead of {@link org.junit.jupiter.api.Test}.
+ *     Note: This doesn't work with {@link org.junit.jupiter.params.ParameterizedTest}.
+ *     Each {@link org.junit.jupiter.api.TestTemplate} will be run with each of the varying
+ *     {@link com.apple.foundationdb.relational.yamltests.configs.YamlTestConfig}, and passed in a parameter:
+ *     {@link YamlRunner} to run the {@code .yamsql} file.
+ * </p>
+ * <p>
+ *     If a specific test cannot be run with a specific config due to a bug, it can be ignored by adding the
+ *     annotation {@link ExcludeYamlTestConfig}. Ideally, these are short lived, primarily to support adding a config,
+ *     and then fixing some tests that may fail with that config.
+ * </p>
  */
 public class YamlTestExtension implements TestTemplateInvocationContextProvider, BeforeAllCallback, AfterAllCallback {
     private static final Logger logger = LogManager.getLogger(YamlTestExtension.class);
@@ -233,7 +245,7 @@ public List<Extension> getAdditionalExtensions() {
     }
 
     /**
-     * Parameter resolver for the class when injecting the {@link YamlTest.Runner}.
+     * Parameter resolver for the class when injecting the {@link YamlRunner}.
      */
     private static final class ClassParameterResolver implements ParameterResolver {
 
@@ -254,27 +266,12 @@ public ClassParameterResolver(@Nonnull final YamlTestConfig config,
 
         @Override
         public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
-            return parameterContext.getParameter().getType().equals(YamlTest.Runner.class);
+            return parameterContext.getParameter().getType().equals(YamlRunner.class);
         }
 
         @Override
         public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
-            return new YamlTest.Runner() {
-                @Override
-                public void runYamsql(final String fileName) throws Exception {
-                    if (filters != null) {
-                        Assumptions.assumeTrue(filters.filter(config), excludedReason);
-                    }
-                    var yamlRunner = new YamlRunner(fileName, config.createConnectionFactory(),
-                            config.getRunnerOptions());
-                    try {
-                        yamlRunner.run();
-                    } catch (Exception e) {
-                        logger.error("‼️ running test file '{}' was not successful", fileName, e);
-                        throw e;
-                    }
-                }
-            };
+            return new YamlRunner(config.createConnectionFactory(), config.getRunnerOptions());
         }
     }
 }

yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/command/QueryCommand.java

@@ -20,14 +20,10 @@
 
 package com.apple.foundationdb.relational.yamltests.command;
 
-import com.apple.foundationdb.record.TestHelpers;
 import com.apple.foundationdb.record.query.plan.cascades.debug.Debugger;
-import com.apple.foundationdb.record.query.plan.debug.DebuggerWithSymbolTables;
 import com.apple.foundationdb.relational.api.Continuation;
 import com.apple.foundationdb.relational.api.exceptions.RelationalException;
-import com.apple.foundationdb.relational.recordlayer.util.ExceptionUtil;
 import com.apple.foundationdb.relational.util.Assert;
-import com.apple.foundationdb.relational.util.Environment;
 import com.apple.foundationdb.relational.yamltests.CustomYamlConstructor;
 import com.apple.foundationdb.relational.yamltests.Matchers;
 import com.apple.foundationdb.relational.yamltests.YamlConnection;
@@ -151,7 +147,6 @@ void executeInternal(@Nonnull final YamlConnection connection) throws SQLExcepti
 
     private void executeInternal(@Nonnull final YamlConnection connection, boolean checkCache, @Nonnull QueryExecutor executor)
             throws SQLException, RelationalException {
-        enableCascadesDebugger();
         boolean shouldExecute = true;
         boolean queryIsRunning = false;
         Continuation continuation = null;
@@ -179,13 +174,13 @@ private void executeInternal(@Nonnull final YamlConnection connection, boolean c
                 // can result in the explain plan being put into the plan cache, so running without the debugger
                 // can pollute cache and thus interfere with the explain's results
                 Integer finalMaxRows = maxRows;
-                runWithDebugger(() -> executor.execute(connection, null, queryConfig, checkCache, finalMaxRows));
+                executor.execute(connection, null, queryConfig, checkCache, finalMaxRows);
             } else if (QueryConfig.QUERY_CONFIG_EXPLAIN.equals(queryConfig.getConfigName()) || QueryConfig.QUERY_CONFIG_EXPLAIN_CONTAINS.equals(queryConfig.getConfigName())) {
                 Assert.that(!queryIsRunning, "Explain test should not be intermingled with query result tests");
                 // ignore debugger configuration, always set the debugger for explain, so we can always get consistent
                 // results
                 Integer finalMaxRows1 = maxRows;
-                runWithDebugger(() -> executor.execute(connection, null, queryConfig, checkCache, finalMaxRows1));
+                executor.execute(connection, null, queryConfig, checkCache, finalMaxRows1);
             } else if (QueryConfig.QUERY_CONFIG_NO_OP.equals(queryConfig.getConfigName())) {
                 // Do nothing for noop execution.
                 continue;
@@ -233,31 +228,4 @@ static void reportTestFailure(@Nonnull String message, @Nullable Throwable throw
             Assertions.fail(message, throwable);
         }
     }
-
-    private static void runWithDebugger(@Nonnull TestHelpers.DangerousRunnable r) throws SQLException {
-        final var savedDebugger = Debugger.getDebugger();
-        try {
-            Debugger.setDebugger(DebuggerWithSymbolTables.withoutSanityChecks());
-            Debugger.setup();
-            r.run();
-        } catch (Exception t) {
-            throw ExceptionUtil.toRelationalException(t).toSqlException();
-        } finally {
-            Debugger.setDebugger(savedDebugger);
-        }
-    }
-
-    /**
-     * Enables internal Cascades debugger which, among other things, sets plan identifiers in a stable fashion making
-     * it easier to view plans and reproduce planning steps.
-     *
-     * @implNote
-     * This is copied from fdb-relational-core:test subproject to avoid having a dependency just for getting access to it.
-     */
-    private static void enableCascadesDebugger() {
-        if (Debugger.getDebugger() == null && Environment.isDebug()) {
-            Debugger.setDebugger(DebuggerWithSymbolTables.withoutSanityChecks());
-        }
-        Debugger.setup();
-    }
 }

yaml-tests/src/test/java/InitialVersionTest.java

@@ -65,7 +65,8 @@ private void doRunCurrentVersion(String testName) throws Exception {
     }
 
     private void doRun(String testName, YamlConnectionFactory connectionFactory) throws Exception {
-        new YamlRunner("initial-version/" + testName + ".yamsql", connectionFactory, YamlExecutionContext.ContextOptions.EMPTY_OPTIONS).run();
+        new YamlRunner(connectionFactory, YamlExecutionContext.ContextOptions.EMPTY_OPTIONS)
+                .runYamsql("initial-version/" + testName + ".yamsql");
     }
 
     YamlConnectionFactory createConnectionFactory() {

yaml-tests/src/test/java/SupportedVersionTest.java

@@ -58,7 +58,8 @@ static void afterAll() throws Exception {
     }
 
     private void doRun(String fileName) throws Exception {
-        new YamlRunner(fileName, createConnectionFactory(), YamlExecutionContext.ContextOptions.EMPTY_OPTIONS).run();
+        new YamlRunner(createConnectionFactory(), YamlExecutionContext.ContextOptions.EMPTY_OPTIONS)
+                .runYamsql(fileName);
     }
 
     YamlConnectionFactory createConnectionFactory() {