Skip to content

fix: fix flaky test and remove warnings #153

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

Merged
merged 1 commit into from
Apr 16, 2020
Merged

fix: fix flaky test and remove warnings #153

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 12 additions & 20 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITSpannerOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@

package com.google.cloud.spanner.it;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static com.google.common.truth.Truth.assertThat;

import com.google.cloud.spanner.Database;
import com.google.cloud.spanner.DatabaseAdminClient;
Expand All @@ -38,18 +36,15 @@
import org.junit.After;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@Category(ParallelIntegrationTest.class)
@RunWith(JUnit4.class)
public class ITSpannerOptionsTest {
@ClassRule public static IntegrationTestEnv env = new IntegrationTestEnv();
@Rule public ExpectedException expectedException = ExpectedException.none();
private static Database db;

@Before
Expand All @@ -73,7 +68,7 @@ public void testCloseAllThreadsWhenClosingSpanner() throws InterruptedException
// The IT environment has already started some worker threads.
int baseThreadCount = getNumberOfThreadsWithName(SPANNER_THREAD_NAME);
for (int i = 0; i < NUMBER_OF_TEST_RUNS; i++) {
assertThat(getNumberOfThreadsWithName(SPANNER_THREAD_NAME), is(equalTo(baseThreadCount)));
assertThat(getNumberOfThreadsWithName(SPANNER_THREAD_NAME)).isAtMost(baseThreadCount);
// Create Spanner instance.
// We make a copy of the options instance, as SpannerOptions caches any service object
// that has been handed out.
Expand Down Expand Up @@ -106,29 +101,26 @@ public void testCloseAllThreadsWhenClosingSpanner() throws InterruptedException
}
// Check the number of threads after the query. Doing a request should initialize a thread
// pool for the underlying SpannerClient.
assertThat(
getNumberOfThreadsWithName(SPANNER_THREAD_NAME),
is(equalTo(DEFAULT_NUM_CHANNELS * NUM_THREADS_PER_CHANNEL + baseThreadCount)));
assertThat(getNumberOfThreadsWithName(SPANNER_THREAD_NAME))
.isEqualTo(DEFAULT_NUM_CHANNELS * NUM_THREADS_PER_CHANNEL + baseThreadCount);

// Then do a request to the InstanceAdmin service and check the number of threads.
// Doing a request should initialize a thread pool for the underlying InstanceAdminClient.
for (int i2 = 0; i2 < DEFAULT_NUM_CHANNELS * 2; i2++) {
InstanceAdminClient instanceAdminClient = spanner.getInstanceAdminClient();
instanceAdminClient.listInstances();
}
assertThat(
getNumberOfThreadsWithName(SPANNER_THREAD_NAME),
is(equalTo(2 * DEFAULT_NUM_CHANNELS * NUM_THREADS_PER_CHANNEL + baseThreadCount)));
assertThat(getNumberOfThreadsWithName(SPANNER_THREAD_NAME))
.isEqualTo(2 * DEFAULT_NUM_CHANNELS * NUM_THREADS_PER_CHANNEL + baseThreadCount);

// Then do a request to the DatabaseAdmin service and check the number of threads.
// Doing a request should initialize a thread pool for the underlying DatabaseAdminClient.
for (int i2 = 0; i2 < DEFAULT_NUM_CHANNELS * 2; i2++) {
DatabaseAdminClient databaseAdminClient = spanner.getDatabaseAdminClient();
databaseAdminClient.listDatabases(db.getId().getInstanceId().getInstance());
}
assertThat(
getNumberOfThreadsWithName(SPANNER_THREAD_NAME),
is(equalTo(3 * DEFAULT_NUM_CHANNELS * NUM_THREADS_PER_CHANNEL + baseThreadCount)));
assertThat(getNumberOfThreadsWithName(SPANNER_THREAD_NAME))
.isEqualTo(3 * DEFAULT_NUM_CHANNELS * NUM_THREADS_PER_CHANNEL + baseThreadCount);

// Now close the Spanner instance and check whether the threads are shutdown or not.
spanner.close();
Expand All @@ -138,7 +130,7 @@ public void testCloseAllThreadsWhenClosingSpanner() throws InterruptedException
&& watch.elapsed(TimeUnit.SECONDS) < 2) {
Thread.sleep(50L);
}
assertThat(getNumberOfThreadsWithName(SPANNER_THREAD_NAME), is(equalTo(baseThreadCount)));
assertThat(getNumberOfThreadsWithName(SPANNER_THREAD_NAME)).isAtMost(baseThreadCount);
}
}

Expand All @@ -150,10 +142,10 @@ public void testMultipleSpannersFromSameSpannerOptions() throws InterruptedExcep
// Having both in the try-with-resources block is not possible, as it is the same instance.
// One will be closed before the other, and the closing of the second instance would fail.
Spanner spanner2 = options.getService();
assertThat(spanner1 == spanner2, is(true));
assertThat(spanner1).isSameInstanceAs(spanner2);
DatabaseClient client1 = spanner1.getDatabaseClient(db.getId());
DatabaseClient client2 = spanner2.getDatabaseClient(db.getId());
assertThat(client1 == client2, is(true));
assertThat(client1).isSameInstanceAs(client2);
try (ResultSet rs1 =
client1
.singleUse()
Expand All @@ -172,7 +164,7 @@ public void testMultipleSpannersFromSameSpannerOptions() throws InterruptedExcep
&& watch.elapsed(TimeUnit.SECONDS) < 2) {
Thread.sleep(50L);
}
assertThat(getNumberOfThreadsWithName(SPANNER_THREAD_NAME), is(equalTo(baseThreadCount)));
assertThat(getNumberOfThreadsWithName(SPANNER_THREAD_NAME)).isAtMost(baseThreadCount);
}

private int getNumberOfThreadsWithName(String serviceName) {
Expand Down