Skip to content
Permalink
Browse files
fix: fix flaky test and remove warnings (#153)
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 d534e350346b0c9ab8057ede36bc3aac473c0b06
Showing with 12 additions and 20 deletions.
  1. +12 −20 google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITSpannerOptionsTest.java
@@ -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;
@@ -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
@@ -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.
@@ -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();
@@ -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);
}
}

@@ -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()
@@ -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) {

0 comments on commit d534e35

Please sign in to comment.