-
Notifications
You must be signed in to change notification settings - Fork 1.1k
chore(bigquery-jdbc): Force integration tests to use per-run datasets #13278
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ | |
|
|
||
| public class ITConnectionTest { | ||
|
|
||
| private static final String DATASET = "JDBC_CONNECTION_TEST_DATASET"; | ||
| private static String DATASET; | ||
| private static final String DEFAULT_CATALOG = ServiceOptions.getDefaultProjectId(); | ||
| static Random random = new Random(); | ||
| static int randomNumber = random.nextInt(999); | ||
|
|
@@ -53,15 +53,14 @@ public class ITConnectionTest { | |
|
|
||
| @BeforeClass | ||
| public static void beforeClass() throws InterruptedException { | ||
|
|
||
| ITBase.setUpDataset(DATASET); | ||
| DATASET = ITBase.getSharedDataset(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ITBase.setUpTable(DATASET, TABLE_NAME); | ||
| ITBase.setUpProcedure(DATASET, TABLE_NAME); | ||
| } | ||
|
|
||
| @AfterClass | ||
| public static void afterClass() throws InterruptedException { | ||
| ITBase.cleanUp(DATASET); | ||
| // Shared dataset cleanup is handled by shutdown hook | ||
| } | ||
|
|
||
| @Test | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -55,9 +55,9 @@ public class ITDatabaseMetadataTest extends ITBase { | |||||||||||||
| + ";OAUTHTYPE=3"; | ||||||||||||||
| private static final Random random = new Random(); | ||||||||||||||
| private static final int randomNumber = random.nextInt(9999); | ||||||||||||||
| private static final String DATASET = "JDBC_PRESUBMIT_INTEGRATION_DATASET"; | ||||||||||||||
| private static final String DATASET2 = "JDBC_PRESUBMIT_INTEGRATION_DATASET_2"; | ||||||||||||||
| private static final String CONSTRAINTS_DATASET = "JDBC_CONSTRAINTS_TEST_DATASET"; | ||||||||||||||
| private static String DATASET; | ||||||||||||||
| private static String DATASET2; | ||||||||||||||
| private static String CONSTRAINTS_DATASET; | ||||||||||||||
| private static final String CONSTRAINTS_TABLE_NAME = "JDBC_CONSTRAINTS_TEST_TABLE"; | ||||||||||||||
| private static final String CONSTRAINTS_TABLE_NAME2 = "JDBC_CONSTRAINTS_TEST_TABLE2"; | ||||||||||||||
| private static final String CONSTRAINTS_TABLE_NAME3 = "JDBC_CONSTRAINTS_TEST_TABLE3"; | ||||||||||||||
|
|
@@ -70,6 +70,9 @@ public class ITDatabaseMetadataTest extends ITBase { | |||||||||||||
|
|
||||||||||||||
| @BeforeClass | ||||||||||||||
| public static void beforeClass() throws InterruptedException, SQLException { | ||||||||||||||
| DATASET = ITBase.getSharedDataset(); | ||||||||||||||
| DATASET2 = ITBase.getSharedDataset2(); | ||||||||||||||
| CONSTRAINTS_DATASET = ITBase.getSharedDataset(); | ||||||||||||||
|
Comment on lines
+73
to
+75
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By assigning both Using the new
Suggested change
|
||||||||||||||
| // Set up Dataset | ||||||||||||||
| ITBase.setUpTable(DATASET, TABLE_NAME); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -36,18 +36,18 @@ public class ITResultSetMetadataTest { | |||||
| static Random random = new Random(); | ||||||
| static int randomNumber = random.nextInt(999); | ||||||
| private static final String TABLE_NAME = "JDBC_RSMETADATA_TEST_TABLE" + randomNumber; | ||||||
| private static final String DATASET = "JDBC_RSMETADATA_TEST_DATASET"; | ||||||
| private static String DATASET; | ||||||
| private static ResultSetMetaData metaData; | ||||||
|
|
||||||
| @BeforeClass | ||||||
| public static void beforeClass() throws InterruptedException { | ||||||
| ITBase.setUpDataset(DATASET); | ||||||
| DATASET = ITBase.getSharedDataset(); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using
Suggested change
|
||||||
| ITBase.setUpTable(DATASET, TABLE_NAME); | ||||||
| } | ||||||
|
|
||||||
| @AfterClass | ||||||
| public static void afterClass() throws InterruptedException { | ||||||
| ITBase.cleanUp(DATASET); | ||||||
| // Shared dataset cleanup is handled by shutdown hook | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,7 +41,7 @@ | |
|
|
||
| public class ITStatementTest { | ||
| private static final String DEFAULT_CATALOG = ServiceOptions.getDefaultProjectId(); | ||
| private static final String DATASET = "JDBC_STATEMENT_TEST_DATASET"; | ||
| private static String DATASET; | ||
| private static String connectionUrl = | ||
| "jdbc:bigquery://https://www.googleapis.com/bigquery/v2:443;ProjectId=%s;OAuthType=3;Timeout=3600;"; | ||
| private static Random random = new Random(); | ||
|
|
@@ -50,13 +50,13 @@ public class ITStatementTest { | |
|
|
||
| @BeforeClass | ||
| public static void beforeClass() throws InterruptedException { | ||
| ITBase.setUpDataset(DATASET); | ||
| DATASET = ITBase.getSharedDataset(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ITBase.setUpTable(DATASET, TABLE_NAME); | ||
| } | ||
|
|
||
| @AfterClass | ||
| public static void afterClass() throws InterruptedException { | ||
| ITBase.cleanUp(DATASET); | ||
| // Shared dataset cleanup is handled by shutdown hook | ||
| } | ||
|
|
||
| @Test | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sharing a single dataset across multiple concurrent or sequential test classes can lead to test interference, especially for metadata queries (like
DatabaseMetaData.getTables) which list all tables in a dataset. Since tables created by other tests are not cleaned up until JVM shutdown, they will be visible to any test querying the shared dataset.To ensure proper test isolation, we should introduce a
getUniqueDataset()helper method inITBasethat creates a new, isolated dataset for each test class's private tables, while registering it for automatic cleanup on shutdown. This allows test classes to use isolated datasets for their own tables, while still usinggetSharedDataset()for shared resources (like constraints and procedures).