Skip to content

Commit 8cafd99

Browse files
harshachintaharshachinta
committed
chore(spanner): revert unit test changes
1 parent 13b0c65 commit 8cafd99

9 files changed

+45
-128
lines changed

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/MultiplexedSessionDatabaseClient.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,12 @@ private void maybeMarkUnimplementedForRW(SpannerException spannerException) {
309309
}
310310

311311
private void verifyBeginTransactionWithRWOnMultiplexedSession(String sessionName) {
312+
// annotate the explict BeginTransactionRequest with a transaction tag
313+
// "multiplexed-rw-background-begin-txn" to avoid storing this request on mock spanner.
314+
// this is to safeguard other mock spanner tests whose BeginTransaction request count will
315+
// otherwise increase by 1. Modifying the unit tests do not seem valid since this code is
316+
// temporary and will be removed once the read-write on multiplexed session looks stable at
317+
// backend.
312318
BeginTransactionRequest.Builder requestBuilder =
313319
BeginTransactionRequest.newBuilder()
314320
.setSession(sessionName)

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/AbstractMockServerTest.java‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -146,19 +146,4 @@ void addUpdateDdlError() {
146146
.build())
147147
.build());
148148
}
149-
150-
boolean isMultiplexedSessionsEnabledForRW(Spanner spanner) {
151-
if (spanner.getOptions() == null || spanner.getOptions().getSessionPoolOptions() == null) {
152-
return false;
153-
}
154-
return spanner.getOptions().getSessionPoolOptions().getUseMultiplexedSessionForRW();
155-
}
156-
157-
// Increment the total request count by 1 when multiplexed session is enabled for read-write
158-
// transactions..
159-
// This is due to the explicit BeginTransaction RPC that is executed once during multiplexed
160-
// session creation.
161-
protected int mayBeIncrementBeginTransactionRequestsCount(Spanner spanner, int count) {
162-
return isMultiplexedSessionsEnabledForRW(spanner) ? count + 0 : count;
163-
}
164149
}

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1872,6 +1872,10 @@ private Transaction getTemporaryTransactionOrNull(TransactionSelector tx) {
18721872
@Override
18731873
public void beginTransaction(
18741874
BeginTransactionRequest request, StreamObserver<Transaction> responseObserver) {
1875+
// Skip storing the explicit BeginTransactionRequest used to verify read-write transaction
1876+
// server availability on multiplexed sessions.
1877+
// This code will be removed once read-write multiplexed sessions are stable on the backend,
1878+
// hence the temporary trade-off.
18751879
if (!request
18761880
.getRequestOptions()
18771881
.getTransactionTag()

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/MultiplexedSessionDatabaseClientMockServerTest.java‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -599,9 +599,6 @@ public void testMutationUsingWrite() {
599599
List<BeginTransactionRequest> beginTransactionRequests =
600600
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
601601
assertEquals(2, beginTransactionRequests.size());
602-
assertEquals(
603-
mayBeIncrementBeginTransactionRequestsCount(spanner, /* count = */ 2),
604-
beginTransactionRequests.size());
605602
for (BeginTransactionRequest request : beginTransactionRequests) {
606603
// Verify that mutation key is set for mutations-only case in read-write transaction.
607604
assertTrue(request.hasMutationKey());
@@ -893,9 +890,7 @@ public void testAbortedReadWriteTxnUsesPreviousTxnIdOnRetryWithExplicitBegin() {
893890
assertThat(updateCount).isEqualTo(1L);
894891
List<BeginTransactionRequest> beginTransactionRequests =
895892
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
896-
assertEquals(
897-
mayBeIncrementBeginTransactionRequestsCount(spanner, /* count = */ 2),
898-
beginTransactionRequests.size());
893+
assertEquals(2, beginTransactionRequests.size());
899894

900895
// Verify the requests are executed using multiplexed sessions
901896
for (BeginTransactionRequest request : beginTransactionRequests) {
@@ -1127,9 +1122,7 @@ public void testPrecommitTokenForTransactionResponse() {
11271122
// Verify that for mutation only case, a mutation key is set in BeginTransactionRequest.
11281123
List<BeginTransactionRequest> beginTxnRequest =
11291124
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
1130-
assertEquals(
1131-
mayBeIncrementBeginTransactionRequestsCount(spanner, /* count = */ 1),
1132-
beginTxnRequest.size());
1125+
assertEquals(1, beginTxnRequest.size());
11331126
assertTrue(mockSpanner.getSession(beginTxnRequest.get(0).getSession()).getMultiplexed());
11341127
assertTrue(beginTxnRequest.get(0).hasMutationKey());
11351128
assertTrue(beginTxnRequest.get(0).getMutationKey().hasInsert());
@@ -1169,9 +1162,7 @@ public void testMutationOnlyCaseAborted() {
11691162
// Verify that for mutation only case, a mutation key is set in BeginTransactionRequest.
11701163
List<BeginTransactionRequest> beginTransactionRequests =
11711164
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
1172-
assertEquals(
1173-
mayBeIncrementBeginTransactionRequestsCount(spanner, /* count = */ 2),
1174-
beginTransactionRequests.size());
1165+
assertEquals(2, beginTransactionRequests.size());
11751166
// Verify the requests are executed using multiplexed sessions
11761167
for (BeginTransactionRequest request : beginTransactionRequests) {
11771168
assertTrue(mockSpanner.getSession(request.getSession()).getMultiplexed());
@@ -1216,8 +1207,7 @@ public void testMutationOnlyUsingTransactionManager() {
12161207
// Verify that for mutation only case, a mutation key is set in BeginTransactionRequest.
12171208
List<BeginTransactionRequest> beginTransactionRequests =
12181209
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
1219-
assertThat(beginTransactionRequests)
1220-
.hasSize(mayBeIncrementBeginTransactionRequestsCount(spanner, /* count = */ 1));
1210+
assertThat(beginTransactionRequests).hasSize(1);
12211211
BeginTransactionRequest beginTransaction = beginTransactionRequests.get(0);
12221212
assertTrue(mockSpanner.getSession(beginTransaction.getSession()).getMultiplexed());
12231213
assertTrue(beginTransaction.hasMutationKey());
@@ -1250,8 +1240,7 @@ public void testMutationOnlyUsingAsyncRunner() {
12501240
// Verify that the mutation key is set in BeginTransactionRequest
12511241
List<BeginTransactionRequest> beginTransactions =
12521242
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
1253-
assertThat(beginTransactions)
1254-
.hasSize(mayBeIncrementBeginTransactionRequestsCount(spanner, /* count = */ 1));
1243+
assertThat(beginTransactions).hasSize(1);
12551244
BeginTransactionRequest beginTransaction = beginTransactions.get(0);
12561245
assertTrue(beginTransaction.hasMutationKey());
12571246
assertTrue(beginTransaction.getMutationKey().hasDelete());
@@ -1287,8 +1276,7 @@ public void testMutationOnlyUsingAsyncTransactionManager() {
12871276
// Verify that the mutation key is set in BeginTransactionRequest
12881277
List<BeginTransactionRequest> beginTransactions =
12891278
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
1290-
assertThat(beginTransactions)
1291-
.hasSize(mayBeIncrementBeginTransactionRequestsCount(spanner, /* count = */ 1));
1279+
assertThat(beginTransactions).hasSize(1);
12921280
BeginTransactionRequest beginTransaction = beginTransactions.get(0);
12931281
assertTrue(beginTransaction.hasMutationKey());
12941282
assertTrue(beginTransaction.getMutationKey().hasDelete());

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/RetryOnDifferentGrpcChannelMockServerTest.java‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,8 @@ public void testReadWriteTransaction_retriesOnNewChannel() {
143143
transaction.buffer(Mutation.newInsertBuilder("foo").set("id").to(1L).build());
144144
return null;
145145
});
146-
assertEquals(
147-
mayBeIncrementBeginTransactionRequestsCount(spanner, 2),
148-
mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
149146
}
147+
assertEquals(2, mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
150148
List<BeginTransactionRequest> requests =
151149
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
152150
assertNotEquals(requests.get(0).getSession(), requests.get(1).getSession());
@@ -182,9 +180,7 @@ public void testReadWriteTransaction_stopsRetrying() {
182180
assertEquals(ErrorCode.DEADLINE_EXCEEDED, exception.getErrorCode());
183181

184182
int numChannels = spanner.getOptions().getNumChannels();
185-
assertEquals(
186-
mayBeIncrementBeginTransactionRequestsCount(spanner, numChannels),
187-
mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
183+
assertEquals(numChannels, mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
188184
List<BeginTransactionRequest> requests =
189185
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
190186
Set<String> sessions =
@@ -245,9 +241,7 @@ public void testDenyListedChannelIsCleared() {
245241
int numChannels = spanner.getOptions().getNumChannels();
246242
// We should have numChannels BeginTransactionRequests from the first transaction, and 2 from
247243
// the second transaction.
248-
assertEquals(
249-
mayBeIncrementBeginTransactionRequestsCount(spanner, numChannels + 2),
250-
mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
244+
assertEquals(numChannels + 2, mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
251245
List<BeginTransactionRequest> requests =
252246
mockSpanner.getRequestsOfType(BeginTransactionRequest.class);
253247
// The requests should all use different sessions, as deny-listing a session will bring it to
@@ -358,14 +352,11 @@ public void testReadWriteTransaction_withGrpcContextDeadline_doesNotRetry() {
358352
return null;
359353
})));
360354
assertEquals(ErrorCode.DEADLINE_EXCEEDED, exception.getErrorCode());
361-
// A gRPC context deadline will still cause the underlying error handler to try to retry the
362-
// transaction on a new channel, but as the deadline has been exceeded even before those RPCs
363-
// are being executed, the RPC invocation will be skipped, and the error will eventually
364-
// bubble
365-
// up.
366-
assertEquals(
367-
mayBeIncrementBeginTransactionRequestsCount(spanner, 1),
368-
mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
369355
}
356+
// A gRPC context deadline will still cause the underlying error handler to try to retry the
357+
// transaction on a new channel, but as the deadline has been exceeded even before those RPCs
358+
// are being executed, the RPC invocation will be skipped, and the error will eventually bubble
359+
// up.
360+
assertEquals(1, mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
370361
}
371362
}

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AbstractMockServerTest.java‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -321,19 +321,4 @@ boolean isMultiplexedSessionsEnabled(Spanner spanner) {
321321
}
322322
return spanner.getOptions().getSessionPoolOptions().getUseMultiplexedSession();
323323
}
324-
325-
boolean isMultiplexedSessionsEnabledForRW(Spanner spanner) {
326-
if (spanner.getOptions() == null || spanner.getOptions().getSessionPoolOptions() == null) {
327-
return false;
328-
}
329-
return spanner.getOptions().getSessionPoolOptions().getUseMultiplexedSessionForRW();
330-
}
331-
332-
// Increment the total request count by 1 when multiplexed session is enabled for read-write
333-
// transactions..
334-
// This is due to the explicit BeginTransaction RPC that is executed once during multiplexed
335-
// session creation.
336-
protected int mayBeIncrementBeginTransactionRequestsCount(Spanner spanner, int count) {
337-
return isMultiplexedSessionsEnabledForRW(spanner) ? count + 0 : count;
338-
}
339324
}

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/DelayTransactionStartUntilFirstWriteMockServerTest.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -362,9 +362,7 @@ public void testTransactionWithQueryFollowedByMutations() {
362362
ExecuteSqlRequest queryRequest =
363363
mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0);
364364
assertFalse(queryRequest.hasTransaction());
365-
assertEquals(
366-
mayBeIncrementBeginTransactionRequestsCount(connection.getSpanner(), /* count = */ 1),
367-
mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
365+
assertEquals(1, mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
368366
assertEquals(1, mockSpanner.countRequestsOfType(CommitRequest.class));
369367
}
370368
}
@@ -384,9 +382,7 @@ public void testTransactionWithMutationsFollowedByQuery() {
384382
ExecuteSqlRequest queryRequest =
385383
mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0);
386384
assertFalse(queryRequest.hasTransaction());
387-
assertEquals(
388-
mayBeIncrementBeginTransactionRequestsCount(connection.getSpanner(), /* count = */ 1),
389-
mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
385+
assertEquals(1, mockSpanner.countRequestsOfType(BeginTransactionRequest.class));
390386
assertEquals(1, mockSpanner.countRequestsOfType(CommitRequest.class));
391387
}
392388
}

0 commit comments

Comments
 (0)