Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable multi version tests #3047

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+21 −109
Open

Enable multi version tests #3047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
54d1035
Enable tests that require client-server continuations
ohadzeliger Jan 15, 2025
e355da4
Merge from main
ohadzeliger Jan 15, 2025
0aab24e
Enable @BeforeAll in extension
ohadzeliger Jan 15, 2025
e84ceb5
Import
ohadzeliger Jan 15, 2025
1dd4539
Import
ohadzeliger Jan 16, 2025
897bcbc
Retry starting the server in case it took some time to shut down afte…
ohadzeliger Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions ...n/java/com/apple/foundationdb/relational/yamltests/server/RunExternalServerExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -87,7 +86,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the issues with explain changing between versions, I'm not sure that we want to enable this as part of prb yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean in general, or just for the tests that contain explain assertions?

File jar;
if (jarName == null) {
final File externalDirectory = new File(Objects.requireNonNull(System.getProperty(EXTERNAL_SERVER_PROPERTY_NAME)));
Expand All @@ -103,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");
}

Expand Down Expand Up @@ -135,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();
}
}
65 changes: 1 addition & 64 deletions yaml-tests/src/test/java/JDBCYamlIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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")
Expand Down
39 changes: 1 addition & 38 deletions yaml-tests/src/test/java/MultiServerJDBCYamlTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -22,7 +22,6 @@
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.junit.jupiter.api.Nested;
import org.junit.jupiter.api.extension.RegisterExtension;

Expand Down Expand Up @@ -93,40 +92,4 @@ public Set<String> 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();
}
}