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
test: fix Spanner R2DBC flaky integration tests #2874
Conversation
@AfterEach | ||
void cleanupTestInstanceAfterTest() { | ||
log.info("Deleting database {}", testDatabase); | ||
this.spanner.getDatabaseAdminClient().dropDatabase(TEST_INSTANCE, testDatabase); |
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.
If something goes wrong and this cleanup isn't able to run, we'll have an orphaned database that never gets removed. Example of resolving this situation in google-cloud-java, by checking at the beginning of a test whether any stale resources exist from prior runs: https://github.com/googleapis/google-cloud-java/blob/main/java-compute/google-cloud-compute/src/test/java/com/google/cloud/compute/v1/integration/Util.java#L41-L57
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.
@AfterEach
is guaranteed to run even if there is a test failure. Unlike Compute instances, Spanner databases don't incur additional costs, but there is a limit of 100 per instance. I would rather hold off the proactive additional cleanup that will slow down the tests, until it's clear that we need it.
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.
@AfterEach
doesn't run if the machine fails - the exact problem we had with google-cloud-java. I leave it up to you.
/** | ||
* Abstract base for Spanner R2DBC integration tests of {@link SpannerR2dbcDialect}. | ||
*/ | ||
abstract class AbstractBaseSpannerR2dbcIntegrationTest { |
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.
Test logs show a very large number of exceptions due to:
SEVERE: *~*~*~ Previous channel ManagedChannelImpl***logId=467, target=spanner.googleapis.com:443*** was garbage collected without being shut down! ~*~*~*
Make sure to call shutdown()/shutdownNow()
With long stack traces that pollute the results. Can we shut down the clients to avoid these errors?
Quality Gate passedIssues Measures |
Create a new database for each test for isolation.
Fixes: #2832.
Fixes: #2314.