Skip to content

Commit ebe6a3f

Browse files
tdcmeehantdcmeehan
authored
chore(plugin-postgres): Refactor MySQL and Postgres tests to use TestContainers (#26459)
## Description To address [#26442](#26442), migrate our Postgres and MySQL tests to use TestContainers, which is a better supported way to spin up these servers for testing purposes. ## Motivation and Context Fix [#26442](#26442). ## Impact Locally run such tests in Apple Silicon hardware. ## Test Plan CI has been refactored to run these tests in a separate job, as some tests were not running in CI ([#26457](#26457)). ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. - [ ] If adding new dependencies, verified they have an [OpenSSF Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or higher (or obtained explicit TSC approval for lower scores). ## Release Notes ``` == NO RELEASE NOTE == ```
1 parent e024bd8 commit ebe6a3f

16 files changed

+414
-179
lines changed

‎.github/workflows/jdbc-connector-tests.yml‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
name: jdbc connector tests
2+
3+
on: pull_request
4+
5+
env:
6+
# An envar that signals to tests we are executing in the CI environment
7+
CONTINUOUS_INTEGRATION: true
8+
MAVEN_OPTS: -Xmx1024M -XX:+ExitOnOutOfMemoryError
9+
MAVEN_INSTALL_OPTS: -Xmx2G -XX:+ExitOnOutOfMemoryError
10+
MAVEN_FAST_INSTALL: -B -V --quiet -T 1C -DskipTests -Dair.check.skip-all --no-transfer-progress -Dmaven.javadoc.skip=true
11+
MAVEN_TEST: -B -Dair.check.skip-all -Dmaven.javadoc.skip=true -DLogTestDurationListener.enabled=true --no-transfer-progress --fail-at-end
12+
RETRY: .github/bin/retry
13+
14+
jobs:
15+
changes:
16+
runs-on: ubuntu-latest
17+
# Required permissions
18+
permissions:
19+
pull-requests: read
20+
# Set job outputs to values from filter step
21+
outputs:
22+
codechange: ${{ steps.filter.outputs.codechange }}
23+
steps:
24+
# For pull requests it's not necessary to checkout the code
25+
- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
26+
id: filter
27+
with:
28+
filters: |
29+
codechange:
30+
- '!presto-docs/**'
31+
32+
jdbc-connector-tests:
33+
permissions:
34+
contents: read
35+
strategy:
36+
fail-fast: false
37+
matrix:
38+
java: [17.0.15]
39+
runs-on: ubuntu-latest
40+
needs: changes
41+
timeout-minutes: 60
42+
concurrency:
43+
group: ${{ github.workflow }}-jdbc-connector-tests-${{ github.event.pull_request.number }}-${{ matrix.java }}
44+
cancel-in-progress: true
45+
steps:
46+
- uses: actions/checkout@v4
47+
if: needs.changes.outputs.codechange == 'true'
48+
with:
49+
show-progress: false
50+
persist-credentials: false
51+
- uses: actions/setup-java@v4
52+
if: needs.changes.outputs.codechange == 'true'
53+
with:
54+
distribution: temurin
55+
java-version: ${{ matrix.java }}
56+
cache: maven
57+
- name: Download nodejs to maven cache
58+
if: needs.changes.outputs.codechange == 'true'
59+
run: .github/bin/download_nodejs
60+
- name: Install JDBC Connector Modules
61+
if: needs.changes.outputs.codechange == 'true'
62+
run: |
63+
export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
64+
./mvnw install ${MAVEN_FAST_INSTALL} -am -pl :presto-mysql,:presto-postgresql
65+
- name: Run MySQL and PostgreSQL Tests
66+
if: needs.changes.outputs.codechange == 'true'
67+
run: ./mvnw test ${MAVEN_TEST} -P ci -pl :presto-mysql,:presto-postgresql

‎.github/workflows/test-other-modules.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,4 @@ jobs:
6767
./mvnw install ${MAVEN_FAST_INSTALL} -pl '!:presto-docs,!:presto-server,!presto-test-coverage'
6868
- name: Maven Tests
6969
if: needs.changes.outputs.codechange == 'true'
70-
run: ./mvnw test -T 1 ${MAVEN_TEST} -P skip-native-sidecar-tests -pl '!presto-tests, !presto-native-tests, !presto-accumulo, !presto-cassandra, !presto-hive, !presto-kudu, !presto-docs, !presto-server, !presto-main, !presto-main-base, !presto-mongodb, !presto-spark-package, !presto-spark-launcher, !presto-spark-testing, !presto-spark-base, !presto-redis, !presto-elasticsearch, !presto-orc, !presto-thrift-connector, !presto-native-execution, !presto-test-coverage, !presto-iceberg, !presto-singlestore, !presto-base-arrow-flight, !presto-plan-checker-router-plugin'
70+
run: ./mvnw test -T 1 ${MAVEN_TEST} -P skip-native-sidecar-tests -pl '!presto-tests, !presto-native-tests, !presto-accumulo, !presto-cassandra, !presto-hive, !presto-kudu, !presto-docs, !presto-server, !presto-main, !presto-main-base, !presto-mongodb, !presto-spark-package, !presto-spark-launcher, !presto-spark-testing, !presto-spark-base, !presto-redis, !presto-elasticsearch, !presto-orc, !presto-thrift-connector, !presto-native-execution, !presto-test-coverage, !presto-iceberg, !presto-singlestore, !presto-base-arrow-flight, !presto-plan-checker-router-plugin, !presto-mysql, !presto-postgresql'

‎presto-mysql/pom.xml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,14 +159,14 @@
159159
</dependency>
160160

161161
<dependency>
162-
<groupId>com.facebook.presto</groupId>
163-
<artifactId>testing-mysql-server-8</artifactId>
162+
<groupId>org.testcontainers</groupId>
163+
<artifactId>testcontainers</artifactId>
164164
<scope>test</scope>
165165
</dependency>
166166

167167
<dependency>
168-
<groupId>com.facebook.presto</groupId>
169-
<artifactId>testing-mysql-server-base</artifactId>
168+
<groupId>org.testcontainers</groupId>
169+
<artifactId>mysql</artifactId>
170170
<scope>test</scope>
171171
</dependency>
172172

‎presto-mysql/src/test/java/com/facebook/presto/plugin/mysql/MySqlQueryRunner.java‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,8 @@
1515

1616
import com.facebook.presto.Session;
1717
import com.facebook.presto.testing.QueryRunner;
18-
import com.facebook.presto.testing.mysql.TestingMySqlServer;
1918
import com.facebook.presto.tests.DistributedQueryRunner;
2019
import com.facebook.presto.tpch.TpchPlugin;
21-
import com.google.common.collect.ImmutableList;
2220
import com.google.common.collect.ImmutableMap;
2321
import io.airlift.tpch.TpchTable;
2422

@@ -38,25 +36,13 @@ private MySqlQueryRunner()
3836

3937
private static final String TPCH_SCHEMA = "tpch";
4038

41-
public static QueryRunner createMySqlQueryRunner(TestingMySqlServer server, TpchTable<?>... tables)
39+
public static QueryRunner createMySqlQueryRunner(String jdbcUrl, Map<String, String> connectorProperties, Iterable<TpchTable<?>> tables)
4240
throws Exception
4341
{
44-
return createMySqlQueryRunner(server, ImmutableMap.of(), ImmutableList.copyOf(tables));
42+
return createMySqlQueryRunner(jdbcUrl, connectorProperties, tables, "testuser", "testpass");
4543
}
4644

47-
public static QueryRunner createMySqlQueryRunner(TestingMySqlServer server, Map<String, String> connectorProperties, Iterable<TpchTable<?>> tables)
48-
throws Exception
49-
{
50-
try {
51-
return createMySqlQueryRunner(server.getJdbcUrl(), connectorProperties, tables);
52-
}
53-
catch (Throwable e) {
54-
closeAllSuppress(e, server);
55-
throw e;
56-
}
57-
}
58-
59-
public static QueryRunner createMySqlQueryRunner(String jdbcUrl, Map<String, String> connectorProperties, Iterable<TpchTable<?>> tables)
45+
public static QueryRunner createMySqlQueryRunner(String jdbcUrl, Map<String, String> connectorProperties, Iterable<TpchTable<?>> tables, String username, String password)
6046
throws Exception
6147
{
6248
DistributedQueryRunner queryRunner = null;
@@ -66,8 +52,12 @@ public static QueryRunner createMySqlQueryRunner(String jdbcUrl, Map<String, Str
6652
queryRunner.installPlugin(new TpchPlugin());
6753
queryRunner.createCatalog("tpch", "tpch");
6854

55+
String jdbcUrlWithoutDatabase = removeDatabaseFromJdbcUrl(jdbcUrl);
56+
String jdbcUrlWithCredentials = jdbcUrlWithoutDatabase + (jdbcUrlWithoutDatabase.contains("?") ? "&" : "?") +
57+
"user=" + username + "&password=" + password;
58+
6959
connectorProperties = new HashMap<>(ImmutableMap.copyOf(connectorProperties));
70-
connectorProperties.putIfAbsent("connection-url", jdbcUrl);
60+
connectorProperties.putIfAbsent("connection-url", jdbcUrlWithCredentials);
7161
connectorProperties.putIfAbsent("allow-drop-table", "true");
7262

7363
queryRunner.installPlugin(new MySqlPlugin());
@@ -90,4 +80,9 @@ public static Session createSession()
9080
.setSchema(TPCH_SCHEMA)
9181
.build();
9282
}
83+
84+
public static String removeDatabaseFromJdbcUrl(String jdbcUrl)
85+
{
86+
return jdbcUrl.replaceFirst("/[^/?]+([?]|$)", "/$1");
87+
}
9388
}

‎presto-mysql/src/test/java/com/facebook/presto/plugin/mysql/TestCredentialPassthrough.java‎

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,47 +16,76 @@
1616
import com.facebook.presto.Session;
1717
import com.facebook.presto.spi.security.Identity;
1818
import com.facebook.presto.testing.QueryRunner;
19-
import com.facebook.presto.testing.mysql.TestingMySqlServer;
2019
import com.facebook.presto.tests.DistributedQueryRunner;
2120
import com.google.common.collect.ImmutableMap;
21+
import org.testcontainers.containers.MySQLContainer;
22+
import org.testng.annotations.AfterClass;
2223
import org.testng.annotations.Test;
2324

25+
import java.sql.Connection;
26+
import java.sql.DriverManager;
27+
import java.sql.SQLException;
28+
import java.sql.Statement;
2429
import java.util.Map;
2530
import java.util.Optional;
2631

2732
import static com.facebook.airlift.testing.Closeables.closeAllSuppress;
33+
import static com.facebook.presto.plugin.mysql.MySqlQueryRunner.removeDatabaseFromJdbcUrl;
2834
import static com.facebook.presto.testing.TestingSession.testSessionBuilder;
2935
import static java.lang.String.format;
3036

3137
public class TestCredentialPassthrough
3238
{
3339
private static final String TEST_SCHEMA = "test_database";
34-
private final TestingMySqlServer mysqlServer;
40+
private static final String TEST_USER = "testuser";
41+
private static final String TEST_PASSWORD = "testpass";
42+
43+
private final MySQLContainer<?> mysqlContainer;
3544
private final QueryRunner mySqlQueryRunner;
3645

3746
public TestCredentialPassthrough()
3847
throws Exception
3948
{
40-
mysqlServer = new TestingMySqlServer("testuser", "testpass", TEST_SCHEMA);
41-
mySqlQueryRunner = createQueryRunner(mysqlServer);
49+
mysqlContainer = new MySQLContainer<>("mysql:8.0")
50+
.withDatabaseName(TEST_SCHEMA)
51+
.withUsername(TEST_USER)
52+
.withPassword(TEST_PASSWORD);
53+
mysqlContainer.start();
54+
55+
try (Connection connection = DriverManager.getConnection(mysqlContainer.getJdbcUrl(), TEST_USER, TEST_PASSWORD);
56+
Statement statement = connection.createStatement()) {
57+
statement.execute("CREATE DATABASE IF NOT EXISTS " + TEST_SCHEMA);
58+
}
59+
catch (SQLException e) {
60+
throw new RuntimeException("Failed to create " + TEST_SCHEMA, e);
61+
}
62+
63+
mySqlQueryRunner = createQueryRunner(mysqlContainer);
64+
}
65+
66+
@AfterClass(alwaysRun = true)
67+
public void destroy()
68+
{
69+
if (mysqlContainer != null) {
70+
mysqlContainer.stop();
71+
}
4272
}
4373

4474
@Test
4575
public void testCredentialPassthrough()
46-
throws Exception
4776
{
48-
mySqlQueryRunner.execute(getSession(mysqlServer), "CREATE TABLE test_create (a bigint, b double, c varchar)");
77+
mySqlQueryRunner.execute(getSession(mysqlContainer), "CREATE TABLE test_create (a bigint, b double, c varchar)");
4978
}
5079

51-
public static QueryRunner createQueryRunner(TestingMySqlServer mySqlServer)
80+
public static QueryRunner createQueryRunner(MySQLContainer<?> mysqlContainer)
5281
throws Exception
5382
{
5483
DistributedQueryRunner queryRunner = null;
5584
try {
5685
queryRunner = DistributedQueryRunner.builder(testSessionBuilder().build()).build();
5786
queryRunner.installPlugin(new MySqlPlugin());
5887
Map<String, String> properties = ImmutableMap.<String, String>builder()
59-
.put("connection-url", getConnectionUrl(mySqlServer))
88+
.put("connection-url", getConnectionUrl(mysqlContainer))
6089
.put("user-credential-name", "mysql.user")
6190
.put("password-credential-name", "mysql.password")
6291
.build();
@@ -65,19 +94,19 @@ public static QueryRunner createQueryRunner(TestingMySqlServer mySqlServer)
6594
return queryRunner;
6695
}
6796
catch (Exception e) {
68-
closeAllSuppress(e, queryRunner, mySqlServer);
97+
closeAllSuppress(e, queryRunner);
6998
throw e;
7099
}
71100
}
72101

73-
private static Session getSession(TestingMySqlServer mySqlServer)
102+
private static Session getSession(MySQLContainer<?> mysqlContainer)
74103
{
75-
Map<String, String> extraCredentials = ImmutableMap.of("mysql.user", mySqlServer.getUser(), "mysql.password", mySqlServer.getPassword());
104+
Map<String, String> extraCredentials = ImmutableMap.of("mysql.user", mysqlContainer.getUsername(), "mysql.password", mysqlContainer.getPassword());
76105
return testSessionBuilder()
77106
.setCatalog("mysql")
78107
.setSchema(TEST_SCHEMA)
79108
.setIdentity(new Identity(
80-
mySqlServer.getUser(),
109+
mysqlContainer.getUsername(),
81110
Optional.empty(),
82111
ImmutableMap.of(),
83112
extraCredentials,
@@ -87,8 +116,9 @@ private static Session getSession(TestingMySqlServer mySqlServer)
87116
.build();
88117
}
89118

90-
private static String getConnectionUrl(TestingMySqlServer mySqlServer)
119+
private static String getConnectionUrl(MySQLContainer<?> mysqlContainer)
91120
{
92-
return format("jdbc:mysql://localhost:%s?useSSL=false&allowPublicKeyRetrieval=true", mySqlServer.getPort());
121+
String jdbcUrlWithoutDatabase = removeDatabaseFromJdbcUrl(mysqlContainer.getJdbcUrl());
122+
return format("%s?useSSL=false&allowPublicKeyRetrieval=true", jdbcUrlWithoutDatabase.split("\\?")[0]);
93123
}
94124
}

0 commit comments

Comments
 (0)