From 54d1035fc710a98f279b3a42338ead7ee9ab238b Mon Sep 17 00:00:00 2001 From: ohad Date: Wed, 15 Jan 2025 14:43:44 -0500 Subject: [PATCH 1/6] Enable tests that require client-server continuations --- .../test/java/JDBCYamlIntegrationTests.java | 65 +----------------- .../test/java/MultiServerJDBCYamlTests.java | 66 ++++++++----------- 2 files changed, 28 insertions(+), 103 deletions(-) diff --git a/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java b/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java index 34449a5831..1ce0731857 100644 --- a/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java +++ b/yaml-tests/src/test/java/JDBCYamlIntegrationTests.java @@ -3,7 +3,7 @@ * * This source file is part of the FoundationDB open source project * - * Copyright 2021-2024 Apple Inc. and the FoundationDB project authors + * Copyright 2015-2025 Apple Inc. and the FoundationDB project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,69 +27,6 @@ */ public abstract class JDBCYamlIntegrationTests extends YamlIntegrationTests { - @Override - @Test - @Disabled("TODO: Flakey") - public void orderBy() throws Exception { - super.orderBy(); - } - - @Override - @Test - @Disabled("TODO: Flakey") - public void scenarioTests() throws Exception { - super.scenarioTests(); - } - - @Override - @Test - @Disabled("Requires continuation support") - public void versionsTests() throws Exception { - super.versionsTests(); - } - - @Override - @Test - @Disabled("TODO: Need to work on supporting labels") - public void maxRows() throws Exception { - super.maxRows(); - } - - @Override - @Test - @Disabled("TODO") - public void selectAStar() throws Exception { - super.selectAStar(); - } - - @Override - @Test - @Disabled("TODO: Flakey") - public void aggregateIndexTestsCount() throws Exception { - super.aggregateIndexTestsCount(); - } - - @Override - @Test - @Disabled("TODO: Flakey") - public void joinTests() throws Exception { - super.joinTests(); - } - - @Override - @Test - @Disabled("TODO: Flakey") - public void nested() throws Exception { - super.nested(); - } - - @Override - @Test - @Disabled("TODO: Flakey") - public void showcasingTests() throws Exception { - super.showcasingTests(); - } - @Override @Test @Disabled("TODO: Not supported") diff --git a/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java b/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java index 7e5ef1c8a4..3f47b27d9b 100644 --- a/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java +++ b/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java @@ -3,7 +3,7 @@ * * This source file is part of the FoundationDB open source project * - * Copyright 2021-2024 Apple Inc. and the FoundationDB project authors + * Copyright 2015-2025 Apple Inc. and the FoundationDB project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,8 +21,11 @@ import com.apple.foundationdb.relational.api.RelationalConnection; import com.apple.foundationdb.relational.yamltests.MultiServerConnectionFactory; import com.apple.foundationdb.relational.yamltests.YamlRunner; -import com.apple.foundationdb.relational.yamltests.server.RunExternalServerExtension; -import org.junit.jupiter.api.Disabled; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.extension.RegisterExtension; @@ -70,6 +73,27 @@ public MultiServerInitialConnectionExternalTests() { } } + @BeforeAll + public static void startServer() throws IOException, InterruptedException { + final File externalDirectory = new File(Objects.requireNonNull(System.getProperty(EXTERNAL_SERVER_PROPERTY_NAME))); + final File[] externalServers = Objects.requireNonNull( + externalDirectory.listFiles(file -> file.getName().endsWith(".jar")), + "No downloaded server jars were found"); + Assertions.assertEquals(1, externalServers.length); + File jar = externalServers[0]; + LOG.info("Starting " + jar); + ProcessBuilder processBuilder = new ProcessBuilder("java", "-jar", jar.getAbsolutePath()); + processBuilder.redirectOutput(ProcessBuilder.Redirect.INHERIT); + processBuilder.redirectError(ProcessBuilder.Redirect.INHERIT); + + serverProcess = processBuilder.start(); + // TODO: There should be a better way to figure out that the server is fully up and running + Thread.sleep(3000); + if ((serverProcess == null) || !serverProcess.isAlive()) { + Assertions.fail("Failed to start the external server"); + } + } + @Override YamlRunner.YamlConnectionFactory createConnectionFactory() { return new MultiServerConnectionFactory( @@ -93,40 +117,4 @@ public Set getVersionsUnderTest() { } }; } - - @Override - @Disabled("Test asserts about quantifiers") - public void updateDeleteReturning() throws Exception { - super.updateDeleteReturning(); - } - - @Override - @Disabled("Test asserts about quantifiers") - public void indexedFunctions() throws Exception { - super.indexedFunctions(); - } - - @Override - @Disabled("Test asserts about quantifiers") - public void bitmap() throws Exception { - super.bitmap(); - } - - @Override - @Disabled("Test asserts about quantifiers") - public void cte() throws Exception { - super.cte(); - } - - @Override - @Disabled("Test asserts about quantifiers") - public void unionEmptyTables() throws Exception { - super.unionEmptyTables(); - } - - @Override - @Disabled("Test asserts about quantifiers") - public void union() throws Exception { - super.union(); - } } From e355da4d0d3f8826a6a90a2d131b3ffecdaf403a Mon Sep 17 00:00:00 2001 From: ohad Date: Wed, 15 Jan 2025 14:54:24 -0500 Subject: [PATCH 2/6] Merge from main --- .../test/java/MultiServerJDBCYamlTests.java | 30 ++++--------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java b/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java index 3f47b27d9b..3f73612553 100644 --- a/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java +++ b/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java @@ -21,21 +21,24 @@ import com.apple.foundationdb.relational.api.RelationalConnection; import com.apple.foundationdb.relational.yamltests.MultiServerConnectionFactory; import com.apple.foundationdb.relational.yamltests.YamlRunner; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.junit.jupiter.api.AfterAll; +import com.apple.foundationdb.relational.yamltests.server.RunExternalServerExtension; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.extension.RegisterExtension; import javax.annotation.Nonnull; +import java.io.File; +import java.io.IOException; import java.net.URI; import java.sql.DriverManager; import java.sql.SQLException; import java.util.List; +import java.util.Objects; import java.util.Set; +import static com.apple.foundationdb.relational.yamltests.server.RunExternalServerExtension.EXTERNAL_SERVER_PROPERTY_NAME; + /** * A test runner to launch the YAML tests with multiple servers. * The tests will run against two servers: One JDBC embedded and the other launched from a jar, that represents another version @@ -73,27 +76,6 @@ public MultiServerInitialConnectionExternalTests() { } } - @BeforeAll - public static void startServer() throws IOException, InterruptedException { - final File externalDirectory = new File(Objects.requireNonNull(System.getProperty(EXTERNAL_SERVER_PROPERTY_NAME))); - final File[] externalServers = Objects.requireNonNull( - externalDirectory.listFiles(file -> file.getName().endsWith(".jar")), - "No downloaded server jars were found"); - Assertions.assertEquals(1, externalServers.length); - File jar = externalServers[0]; - LOG.info("Starting " + jar); - ProcessBuilder processBuilder = new ProcessBuilder("java", "-jar", jar.getAbsolutePath()); - processBuilder.redirectOutput(ProcessBuilder.Redirect.INHERIT); - processBuilder.redirectError(ProcessBuilder.Redirect.INHERIT); - - serverProcess = processBuilder.start(); - // TODO: There should be a better way to figure out that the server is fully up and running - Thread.sleep(3000); - if ((serverProcess == null) || !serverProcess.isAlive()) { - Assertions.fail("Failed to start the external server"); - } - } - @Override YamlRunner.YamlConnectionFactory createConnectionFactory() { return new MultiServerConnectionFactory( From 0aab24e282668a2c76b2aefdf0a30d78f8438981 Mon Sep 17 00:00:00 2001 From: ohad Date: Wed, 15 Jan 2025 15:39:05 -0500 Subject: [PATCH 3/6] Enable @BeforeAll in extension --- .../relational/yamltests/server/RunExternalServerExtension.java | 1 - 1 file changed, 1 deletion(-) diff --git a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java index a8d490ce8a..95f1a05098 100644 --- a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java +++ b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java @@ -87,7 +87,6 @@ public String getVersion() { @Override public void beforeAll(ExtensionContext context) throws Exception { - Assumptions.abort(); // Will be able to re-enable when we have a published external server to use here File jar; if (jarName == null) { final File externalDirectory = new File(Objects.requireNonNull(System.getProperty(EXTERNAL_SERVER_PROPERTY_NAME))); From e84ceb50f0c922bca8800214b1c8b97bf4f92b51 Mon Sep 17 00:00:00 2001 From: ohad Date: Wed, 15 Jan 2025 16:49:00 -0500 Subject: [PATCH 4/6] Import --- .../relational/yamltests/server/RunExternalServerExtension.java | 1 - 1 file changed, 1 deletion(-) diff --git a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java index 95f1a05098..1d2f09793e 100644 --- a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java +++ b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java @@ -23,7 +23,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.ExtensionContext; From 1dd4539fe101a9cb1c09f051e65865a2c7fa67d5 Mon Sep 17 00:00:00 2001 From: ohad Date: Wed, 15 Jan 2025 21:35:58 -0500 Subject: [PATCH 5/6] Import --- yaml-tests/src/test/java/MultiServerJDBCYamlTests.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java b/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java index 3f73612553..7d31cdc411 100644 --- a/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java +++ b/yaml-tests/src/test/java/MultiServerJDBCYamlTests.java @@ -22,23 +22,16 @@ import com.apple.foundationdb.relational.yamltests.MultiServerConnectionFactory; import com.apple.foundationdb.relational.yamltests.YamlRunner; import com.apple.foundationdb.relational.yamltests.server.RunExternalServerExtension; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.extension.RegisterExtension; import javax.annotation.Nonnull; -import java.io.File; -import java.io.IOException; import java.net.URI; import java.sql.DriverManager; import java.sql.SQLException; import java.util.List; -import java.util.Objects; import java.util.Set; -import static com.apple.foundationdb.relational.yamltests.server.RunExternalServerExtension.EXTERNAL_SERVER_PROPERTY_NAME; - /** * A test runner to launch the YAML tests with multiple servers. * The tests will run against two servers: One JDBC embedded and the other launched from a jar, that represents another version From 897bcbc4022d73a85c5753cccb87fc376a647b0a Mon Sep 17 00:00:00 2001 From: ohad Date: Thu, 23 Jan 2025 12:53:32 -0500 Subject: [PATCH 6/6] Retry starting the server in case it took some time to shut down after the previous test run --- .../server/RunExternalServerExtension.java | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java index 1d2f09793e..c3d0416d6c 100644 --- a/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java +++ b/yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java @@ -101,11 +101,7 @@ public void beforeAll(ExtensionContext context) throws Exception { processBuilder.redirectOutput(ProcessBuilder.Redirect.INHERIT); processBuilder.redirectError(ProcessBuilder.Redirect.INHERIT); - this.serverProcess = processBuilder.start(); - - // TODO: There should be a better way to figure out that the server is fully up and running - Thread.sleep(3000); - if (!serverProcess.isAlive()) { + if (!startServer(processBuilder)) { Assertions.fail("Failed to start the external server"); } @@ -133,4 +129,22 @@ public void afterAll(ExtensionContext context) throws Exception { } } + private boolean startServer(ProcessBuilder processBuilder) throws IOException, InterruptedException { + try { + serverProcess = processBuilder.start(); + // TODO: There should be a better way to figure out that the server is fully up and running + Thread.sleep(3000); + if (!serverProcess.isAlive()) { + throw new Exception("Failed to start server once - retrying"); + } + return true; + } catch (Exception ex) { + // Try once more + serverProcess = processBuilder.start(); + // TODO: There should be a better way to figure out that the server is fully up and running + Thread.sleep(3000); + } + + return serverProcess.isAlive(); + } }