test: removes flaky close result set test (#888)

thiagotnunes · web-flow · commit b2b7bb20a704 · 2021-02-19T09:38:42.000+11:00
This test was introduced in order to debug stuck transactions. The cause was unrelated though.
The test here is flaky, because there is a race condition between setting the stream field in the AbstractResultSet and checking it in the close method. Since the result set is not intended to be thread safe, we will not be synchronizing this variable.
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InlineBeginTransactionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InlineBeginTransactionTest.java
@@ -36,7 +36,6 @@
 import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
 import com.google.cloud.spanner.TransactionRunner.TransactionCallable;
 import com.google.cloud.spanner.TransactionRunnerImpl.TransactionContextImpl;
-import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.protobuf.AbstractMessage;
@@ -61,15 +60,13 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.Callable;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import org.junit.After;
@@ -1501,76 +1498,6 @@ public Void run(TransactionContext transaction) throws Exception {
       assertThat(countRequests(CommitRequest.class)).isEqualTo(1);
     }
 
-    @Test
-    public void testCloseResultSetWhileRequestInFlight() throws Exception {
-      DatabaseClient client = spanner.getDatabaseClient(DatabaseId.of("p", "i", "d"));
-      final ExecutorService service = Executors.newSingleThreadExecutor();
-      try {
-        client
-            .readWriteTransaction()
-            .run(
-                new TransactionCallable<Void>() {
-                  @Override
-                  public Void run(TransactionContext transaction) throws Exception {
-                    final ResultSet rs = transaction.executeQuery(SELECT1);
-                    // Prevent the server from executing the query.
-                    final CountDownLatch latch = new CountDownLatch(1);
-                    mockSpanner.freeze();
-                    service.submit(
-                        new Runnable() {
-                          @Override
-                          public void run() {
-                            try {
-                              // This call will be stuck on the server until the mock server is
-                              // unfrozen.
-                              rs.next();
-                            } finally {
-                              latch.countDown();
-                            }
-                          }
-                        });
-
-                    // First wait for the request to be on the server and then close the result set
-                    // while the request is in flight.
-                    mockSpanner.waitForRequestsToContain(
-                        new Predicate<AbstractMessage>() {
-                          @Override
-                          public boolean apply(AbstractMessage input) {
-                            return input instanceof ExecuteSqlRequest
-                                && ((ExecuteSqlRequest) input).getTransaction().hasBegin();
-                          }
-                        },
-                        1000L);
-                    rs.close();
-                    // The next statement should now fail before it is sent to the server because
-                    // the first statement failed to return a transaction while the result set was
-                    // still open.
-                    mockSpanner.unfreeze();
-                    latch.await(1L, TimeUnit.SECONDS);
-                    try {
-                      transaction.executeUpdate(UPDATE_STATEMENT);
-                      fail("missing expected exception");
-                    } catch (SpannerException e) {
-                      assertThat(e.getErrorCode()).isEqualTo(ErrorCode.FAILED_PRECONDITION);
-                      assertThat(e.getMessage())
-                          .contains("ResultSet was closed before a transaction id was returned");
-                    }
-                    return null;
-                  }
-                });
-        fail("missing expected exception");
-      } catch (SpannerException e) {
-        // The commit request will also fail, which means that the entire transaction will fail.
-        assertThat(e.getErrorCode()).isEqualTo(ErrorCode.FAILED_PRECONDITION);
-        assertThat(e.getMessage())
-            .contains("ResultSet was closed before a transaction id was returned");
-      }
-      service.shutdown();
-      assertThat(countRequests(BeginTransactionRequest.class)).isEqualTo(0);
-      assertThat(countRequests(ExecuteSqlRequest.class)).isEqualTo(1);
-      assertThat(countRequests(CommitRequest.class)).isEqualTo(0);
-    }
-
     @Test
     public void testQueryWithInlineBeginDidNotReturnTransaction() {
       DatabaseClient client = spanner.getDatabaseClient(DatabaseId.of("p", "i", "d"));
