Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(test): testThrottlingBlocking flakyness fix (#1775)
* fix(test): testThrottlingBlocking flakyness fix

When BatcherImpl.sendOutstanding (method that calls callContext.withOption) is called from PushCurrentBatchRunnable, it uses
the batcher's own executor, causing the mockito capture to fail due to be in a different
thread from the one that originated add(). Other times, sendOutstanding will be called from the parent
thread (successful test, in this case from our newFixedThreadPool).
ALthough this is expected behavior, the argument captors do not work
when used in different threads. Mockito.verify() is the recommended way
of dealing with argument captors and happened to be the fix
Internally, verify() relies on thread safe(r) notifiers for when the captors have been interacted with.
When verifying flakiness, 100 runs in two threads were used with
bazel test //gax:com.google.api.gax.batching.BatcherImplTest --runs_per_test=100 --test_filter Throttling --jobs 2

Mockito.verify() implementation:
https://github.com/mockito/mockito/blob/2ded10ec704f2c93648d976031c2520a5a9b84aa/src/main/java/org/mockito/internal/MockitoCore.java#L183

* fix(test): remove unnecessary timeout in verify
  • Loading branch information
diegomarquezp committed Aug 22, 2022
1 parent f0fd491 commit e69393c
Showing 1 changed file with 6 additions and 1 deletion.
Expand Up @@ -887,10 +887,15 @@ public void run() {
}

try {
future.get(100, TimeUnit.MILLISECONDS);
future.get(3, TimeUnit.SECONDS);
} catch (TimeoutException e) {
assertWithMessage("adding elements to batcher should not be blocked").fail();
}

// Mockito recommends using verify() as the ONLY way to interact with Argument
// captors - otherwise it may incur in unexpected behaviour
Mockito.verify(callContext).withOption(key.capture(), value.capture());

// Verify that throttled time is recorded in ApiCallContext
assertThat(key.getValue()).isSameInstanceAs(Batcher.THROTTLED_TIME_KEY);
assertThat(value.getValue()).isAtLeast(throttledTime);
Expand Down

0 comments on commit e69393c

Please sign in to comment.