Skip to content

Commit

Permalink
fix: fix flaky test and remove warnings (#153)
Browse files Browse the repository at this point in the history
The test should check for at most baseThreadCount instead of equality,
as it is possible that the thread pool shuts down one or more of its
threads temporarily.

Fixes #146
  • Loading branch information
olavloite committed Apr 16, 2020
1 parent 3fe3ae0 commit d534e35
Showing 1 changed file with 12 additions and 20 deletions.
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

0 comments on commit d534e35

Please sign in to comment.