chore(bigquery-jdbc): Force integration tests to use per-run datasets#13278
chore(bigquery-jdbc): Force integration tests to use per-run datasets#13278logachev wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the integration tests to use dynamically generated shared datasets instead of hardcoded dataset names, introducing automatic cleanup via JVM shutdown hooks. The reviewer identified that sharing a single dataset across multiple test classes can lead to test interference and metadata query pollution. To resolve this, the reviewer suggested introducing a getUniqueDataset() helper method in ITBase to provide isolated datasets for test classes that do not need to share resources.
| return sharedDataset2; | ||
| } |
There was a problem hiding this comment.
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 in ITBase that 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 using getSharedDataset() for shared resources (like constraints and procedures).
return sharedDataset2;
}
public static String getUniqueDataset() {
String uniqueDataset = "JDBC_IT_UNIQUE_" + System.currentTimeMillis() + "_" + (100 + new java.util.Random().nextInt(900));
try {
setUpDataset(uniqueDataset);
registerShutdownHook(uniqueDataset);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new RuntimeException(e);
}
return uniqueDataset;
}| DATASET = ITBase.getSharedDataset(); | ||
| DATASET2 = ITBase.getSharedDataset2(); | ||
| CONSTRAINTS_DATASET = ITBase.getSharedDataset(); |
There was a problem hiding this comment.
By assigning both DATASET and CONSTRAINTS_DATASET to ITBase.getSharedDataset(), they point to the exact same dataset. This causes the constraints tables and the metadata test tables to coexist in the same dataset, which can interfere with metadata queries that expect these datasets to be isolated.
Using the new getUniqueDataset() helper ensures that DATASET and DATASET2 are isolated, unique datasets for this test class, while CONSTRAINTS_DATASET correctly points to the shared dataset containing the pre-created constraints tables.
| DATASET = ITBase.getSharedDataset(); | |
| DATASET2 = ITBase.getSharedDataset2(); | |
| CONSTRAINTS_DATASET = ITBase.getSharedDataset(); | |
| DATASET = ITBase.getUniqueDataset(); | |
| DATASET2 = ITBase.getUniqueDataset(); | |
| CONSTRAINTS_DATASET = ITBase.getSharedDataset(); |
| @BeforeClass | ||
| public static void beforeClass() throws InterruptedException { | ||
| ITBase.setUpDataset(DATASET); | ||
| DATASET = ITBase.getSharedDataset(); |
There was a problem hiding this comment.
Using getSharedDataset() here means ITResultSetMetadataTest shares its dataset with other test classes. Any tables created here will persist until JVM shutdown and could interfere with metadata queries in other tests. Using getUniqueDataset() ensures complete isolation.
| DATASET = ITBase.getSharedDataset(); | |
| DATASET = ITBase.getUniqueDataset(); |
| @BeforeClass | ||
| public static void beforeClass() throws InterruptedException { | ||
| ITBase.setUpDataset(DATASET); | ||
| DATASET = ITBase.getSharedDataset(); |
| public static void beforeClass() throws InterruptedException { | ||
|
|
||
| ITBase.setUpDataset(DATASET); | ||
| DATASET = ITBase.getSharedDataset(); |
There was a problem hiding this comment.
Currently tests rely on existing datasets. Some of them also try to create tables without unique names (e.g. FakeTable) which results in failures if two integration tests are running concurrently.
This change eliminates dependency on
JDBC_INTEGRATION_DATASETandJDBC_INTEGRATION_DATASET_2and relies on the new unique dataset that is removed up after the run.This will also allow us not to rely on table cleanup as the entire dataset will be removed.