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

Jump to bottom

test: fix Spanner R2DBC flaky integration tests #2874

Merged
merged 2 commits into from
May 13, 2024
Merged

test: fix Spanner R2DBC flaky integration tests #2874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
27bd9f2
test: fix Spanner R2DBC flaky integration tests
meltsufin May 12, 2024
725a764
close spanner object
meltsufin May 13, 2024
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
78 changes: 78 additions & 0 deletions ...com/google/cloud/spanner/r2dbc/springdata/it/AbstractBaseSpannerR2dbcIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright 2024-2024 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.cloud.spanner.r2dbc.springdata.it;

import com.google.cloud.ServiceOptions;
import com.google.cloud.spanner.Spanner;
import com.google.cloud.spanner.SpannerOptions;
import com.google.cloud.spanner.r2dbc.springdata.SpannerR2dbcDialect;
import java.util.List;
import java.util.UUID;
import org.junit.jupiter.api.AfterEach;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.context.annotation.UserConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;

/**
* Abstract base for Spanner R2DBC integration tests of {@link SpannerR2dbcDialect}.
*/
abstract class AbstractBaseSpannerR2dbcIntegrationTest {
Copy link
Member

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?

private static final Logger log =
LoggerFactory.getLogger(AbstractBaseSpannerR2dbcIntegrationTest.class);
protected static final String PROJECT_NAME =
System.getProperty("gcp.project", ServiceOptions.getDefaultProjectId());
protected static final String DRIVER_NAME = "spanner";
protected static final String TEST_INSTANCE =
System.getProperty("spanner.instance", "reactivetest");
protected String testDatabase;
protected Spanner spanner;
protected ApplicationContextRunner contextRunner;

/** Initializes the integration test environment for the Spanner R2DBC dialect. */
void initializeTestEnvironment(Class configurationClass, String tableDdl) {
this.testDatabase = "testdb_" + UUID.randomUUID().toString().replace('-', '_').substring(0, 20);

this.spanner = SpannerOptions.newBuilder().setProjectId(PROJECT_NAME).build().getService();

if (configurationClass != null) {
this.contextRunner =
new ApplicationContextRunner()
.withPropertyValues("testDatabase=" + testDatabase)
.withConfiguration(
UserConfigurations.of(
configurationClass));
}

log.info("Creating database {}", testDatabase);
spanner.getDatabaseAdminClient().createDatabase(
TEST_INSTANCE, testDatabase, List.of(tableDdl));
log.info("Done creating database {}", testDatabase);
}

void initializeTestEnvironment(String tableDdl) {
initializeTestEnvironment(null, tableDdl);
}

@AfterEach
void cleanupTestDatabaseAfterTest() {
log.info("Deleting database {}", testDatabase);
this.spanner.getDatabaseAdminClient().dropDatabase(TEST_INSTANCE, testDatabase);
Copy link
Member

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

Copy link
Member Author

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.

Copy link
Member

@burkedavison burkedavison May 13, 2024 •
edited
Loading

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.

log.info("Done deleting database {}", testDatabase);
this.spanner.close();
}
}
151 changes: 48 additions & 103 deletions .../cloud/spanner/r2dbc/springdata/it/SpannerR2dbcDialectDateTimeBindingIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,10 @@
import java.time.LocalDateTime;
import java.time.OffsetDateTime;
import java.util.Arrays;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
import org.junit.jupiter.api.extension.ExtendWith;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationContext;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.convert.converter.Converter;
Expand All @@ -52,87 +46,26 @@
import org.springframework.data.r2dbc.config.AbstractR2dbcConfiguration;
import org.springframework.data.r2dbc.convert.R2dbcCustomConversions;
import org.springframework.data.r2dbc.core.R2dbcEntityTemplate;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

/**
* Integration tests verifying the {@link Timestamp},{@link Date} support of {@link
* SpannerR2dbcDialect}.
*/
@EnabledIfSystemProperty(named = "it.spanner", matches = "true")
@ExtendWith(SpringExtension.class)
@ContextConfiguration(
classes = SpannerR2dbcDialectDateTimeBindingIntegrationTest.TestConfiguration.class)
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class SpannerR2dbcDialectDateTimeBindingIntegrationTest {

private static final Logger log =
LoggerFactory.getLogger(SpannerR2dbcDialectDateTimeBindingIntegrationTest.class);

private static final String PROJECT_NAME =
System.getProperty("gcp.project", ServiceOptions.getDefaultProjectId());
private static final String DRIVER_NAME = "spanner";

private static final String TEST_INSTANCE =
System.getProperty("spanner.instance", "reactivetest");

private static final String TEST_DATABASE = System.getProperty("spanner.database", "testdb");

private static final ConnectionFactory connectionFactory =
ConnectionFactories.get(
ConnectionFactoryOptions.builder()
.option(Option.valueOf("project"), ServiceOptions.getDefaultProjectId())
.option(PROJECT, PROJECT_NAME)
.option(DRIVER, DRIVER_NAME)
.option(INSTANCE, TEST_INSTANCE)
.option(DATABASE, TEST_DATABASE)
.build());

@Autowired private R2dbcEntityTemplate r2dbcEntityTemplate;

class SpannerR2dbcDialectDateTimeBindingIntegrationTest extends AbstractBaseSpannerR2dbcIntegrationTest {
/** Initializes the integration test environment for the Spanner R2DBC dialect. */
@BeforeAll
static void initializeTestEnvironment() {
Mono.from(connectionFactory.create())
.flatMap(
c ->
Mono.from(c.createStatement("drop table card").execute())
.doOnSuccess(x -> log.info("Table drop completed."))
.doOnError(
x -> {
if (!x.getMessage().contains("Table not found")) {
log.info("Table drop failed. {}", x.getMessage());
}
})
.onErrorResume(x -> Mono.empty())
.thenReturn(c))
.flatMap(
c ->
Mono.from(
c.createStatement(
"create table card ("
+ " id int64 not null,"
+ " expiry_year int64 not null,"
+ " expiry_month int64 not null,"
+ " issue_date date not null,"
+ " requested_at timestamp not null"
+ ") primary key (id)")
.execute())
.doOnSuccess(x -> log.info("Table creation completed.")))
.block();
}

@AfterAll
static void cleanupTableAfterTest() {
Mono.from(connectionFactory.create())
.flatMap(
c ->
Mono.from(c.createStatement("drop table card").execute())
.doOnSuccess(x -> log.info("Table drop completed."))
.doOnError(x -> log.info("Table drop failed.")))
.block();
@BeforeEach
void initializeTestEnvironment() {
super.initializeTestEnvironment(
SpannerR2dbcDialectDateTimeBindingIntegrationTest.TestConfiguration.class,
"create table card ("
+ " id int64 not null,"
+ " expiry_year int64 not null,"
+ " expiry_month int64 not null,"
+ " issue_date date not null,"
+ " requested_at timestamp not null"
+ ") primary key (id)");
}

@Test
Expand All @@ -145,36 +78,48 @@ void shouldReadWriteDateAndTimestampTypes() {
LocalDate.parse("2021-12-31"),
LocalDateTime.parse("2021-12-15T21:30:10"));

this.r2dbcEntityTemplate
.insert(Card.class)
.using(card)
.then()
.as(StepVerifier::create)
.verifyComplete();

this.r2dbcEntityTemplate
.select(Card.class)
.first()
.as(StepVerifier::create)
.expectNextMatches(
c ->
c.getId() == 1L
&& c.getExpiryYear() == 2022
&& c.getExpiryMonth() == 12
&& c.getIssueDate().equals(LocalDate.parse("2021-12-31"))
&& c.getRequestedAt().equals(LocalDateTime.parse("2021-12-15T21:30:10")))
.verifyComplete();
this.contextRunner.run(ctx -> {
R2dbcEntityTemplate r2dbcEntityTemplate = ctx.getBean(R2dbcEntityTemplate.class);

r2dbcEntityTemplate
.insert(Card.class)
.using(card)
.then()
.as(StepVerifier::create)
.verifyComplete();

r2dbcEntityTemplate
.select(Card.class)
.first()
.as(StepVerifier::create)
.expectNextMatches(
c ->
c.getId() == 1L
&& c.getExpiryYear() == 2022
&& c.getExpiryMonth() == 12
&& c.getIssueDate().equals(LocalDate.parse("2021-12-31"))
&& c.getRequestedAt().equals(LocalDateTime.parse("2021-12-15T21:30:10")))
.verifyComplete();
});
}

/** Register custom converters. */
@Configuration
static class TestConfiguration extends AbstractR2dbcConfiguration {

@Autowired ApplicationContext applicationContext;
@Value("${testDatabase}")
private String testDatabase;

@Override
@Bean
public ConnectionFactory connectionFactory() {
return connectionFactory;
return ConnectionFactories.get(
ConnectionFactoryOptions.builder()
.option(Option.valueOf("project"), ServiceOptions.getDefaultProjectId())
.option(PROJECT, PROJECT_NAME)
.option(DRIVER, DRIVER_NAME)
.option(INSTANCE, TEST_INSTANCE)
.option(DATABASE, testDatabase)
.build());
}

@Bean
Expand Down
76 changes: 15 additions & 61 deletions ...java/com/google/cloud/spanner/r2dbc/springdata/it/SpannerR2dbcDialectIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,19 @@
import static io.r2dbc.spi.ConnectionFactoryOptions.DATABASE;
import static io.r2dbc.spi.ConnectionFactoryOptions.DRIVER;

import com.google.cloud.ServiceOptions;
import com.google.cloud.spanner.r2dbc.springdata.it.entities.President;
import io.r2dbc.spi.Connection;
import io.r2dbc.spi.ConnectionFactories;
import io.r2dbc.spi.ConnectionFactory;
import io.r2dbc.spi.ConnectionFactoryOptions;
import io.r2dbc.spi.Option;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Sort;
import org.springframework.data.domain.Sort.Direction;
import org.springframework.data.r2dbc.core.R2dbcEntityTemplate;
import org.springframework.data.relational.core.query.Query;
import org.springframework.r2dbc.core.DatabaseClient;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

/**
Expand All @@ -51,66 +43,28 @@
* `spanner.database` system properties.
*/
@EnabledIfSystemProperty(named = "it.spanner", matches = "true")
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class SpannerR2dbcDialectIntegrationTest {

private static final Logger logger =
LoggerFactory.getLogger(SpannerR2dbcDialectIntegrationTest.class);

private static final String DRIVER_NAME = "spanner";

private static final String TEST_INSTANCE =
System.getProperty("spanner.instance", "reactivetest");

private static final String TEST_DATABASE =
System.getProperty("spanner.database", "testdb");

private static final ConnectionFactory connectionFactory =
ConnectionFactories.get(ConnectionFactoryOptions.builder()
.option(Option.valueOf("project"), ServiceOptions.getDefaultProjectId())
.option(DRIVER, DRIVER_NAME)
.option(INSTANCE, TEST_INSTANCE)
.option(DATABASE, TEST_DATABASE)
.build());

private DatabaseClient databaseClient;

class SpannerR2dbcDialectIntegrationTest extends AbstractBaseSpannerR2dbcIntegrationTest {
private ConnectionFactory connectionFactory;
private R2dbcEntityTemplate r2dbcEntityTemplate;

/**
* Initializes the integration test environment for the Spanner R2DBC dialect.
*/
@BeforeAll
@BeforeEach
public void initializeTestEnvironment() {
Connection connection = Mono.from(connectionFactory.create()).block();
initializeTestEnvironment("CREATE TABLE PRESIDENT ("
+ " NAME STRING(256) NOT NULL,"
+ " START_YEAR INT64 NOT NULL"
+ ") PRIMARY KEY (NAME)");

this.connectionFactory = ConnectionFactories.get(ConnectionFactoryOptions.builder()
.option(Option.valueOf("project"), PROJECT_NAME)
.option(DRIVER, DRIVER_NAME)
.option(INSTANCE, TEST_INSTANCE)
.option(DATABASE, testDatabase)
.build());

this.r2dbcEntityTemplate = new R2dbcEntityTemplate(connectionFactory);
this.databaseClient = this.r2dbcEntityTemplate.getDatabaseClient();

if (SpannerTestUtils.tableExists(connection, "PRESIDENT")) {
this.databaseClient.sql("DROP TABLE PRESIDENT")
.fetch()
.rowsUpdated()
.block();
}

this.databaseClient.sql(
"CREATE TABLE PRESIDENT ("
+ " NAME STRING(256) NOT NULL,"
+ " START_YEAR INT64 NOT NULL"
+ ") PRIMARY KEY (NAME)")
.fetch()
.rowsUpdated()
.block();
}

@AfterEach
public void cleanupTableAfterTest() {
this.databaseClient
.sql("DELETE FROM PRESIDENT where NAME is not null")
.fetch()
.rowsUpdated()
.block();
}

@Test
Expand Down
Loading
Loading