Skip to content

test(auth): fix flaky refreshCredentialsIfRequired_asyncSingleThread test #14790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
lqiu96 wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

test(auth): fix flaky refreshCredentialsIfRequired_asyncSingleThread test #14790

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55f9f23
test(auth): fix flaky refreshCredentialsIfRequired_asyncSingleThread …
lqiu96 Oct 20, 2026
688a86a
Merge branch 'main' into fix/auth-cab-async-singlethread-12575
lqiu96 Oct 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions ...m/google/auth/credentialaccessboundary/ClientSideCredentialAccessBoundaryFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,24 +283,39 @@ void refreshCredentialsIfRequired_blockingSingleThread() throws IOException {

@Test
void refreshCredentialsIfRequired_asyncSingleThread() throws IOException {
int numRefresh = 3;
// createMockClock() counts down refreshEntryLatch on every clock.currentTimeMillis() call.
// The initial refreshCredentialsIfRequired() call initializes intermediateCredentials without
// querying the clock. Each subsequent call evaluates determineRefreshType() twice (via
// double-checked locking, once outside and once inside synchronized (refreshLock)), consuming
// 2 clock calls per iteration (numRefresh * 2 total).
// Adding 1 extra permit keeps the background STS request paused in
// MockStsTransportFactory.create() until after asserting that requestCount is still 1. The
// remaining permit is released in the finally block.
CountDownLatch refreshEntryLatch = new CountDownLatch(numRefresh * 2 + 1);
final ClientSideCredentialAccessBoundaryFactory factory =
getClientSideCredentialAccessBoundaryFactory(RefreshType.ASYNC);
getClientSideCredentialAccessBoundaryFactory(RefreshType.ASYNC, refreshEntryLatch);

// Call refreshCredentialsIfRequired() once to initialize the intermediate credentials. This
// should make one request to the STS endpoint.
factory.refreshCredentialsIfRequired();
assertEquals(1, mockStsTransportFactory.transport.getRequestCount());

// Subsequent calls to refreshCredentialsIfRequired() in an async mode should NOT
// immediately call the STS endpoint. They should schedule an asynchronous refresh.
int numRefresh = 3;
for (int i = 0; i < numRefresh; i++) {
factory.refreshCredentialsIfRequired();
}
try {
// Subsequent calls to refreshCredentialsIfRequired() in an async mode should NOT
// immediately call the STS endpoint. They should schedule an asynchronous refresh.
for (int i = 0; i < numRefresh; i++) {
factory.refreshCredentialsIfRequired();
}

// Verify that only the initial call resulted in an immediate STS request. The async refresh
// is still pending.
assertEquals(1, mockStsTransportFactory.transport.getRequestCount());
// Verify that only the initial call resulted in an immediate STS request. The async refresh
// is still pending.
assertEquals(1, mockStsTransportFactory.transport.getRequestCount());
} finally {
while (refreshEntryLatch.getCount() > 0) {
refreshEntryLatch.countDown();
}
}
Comment on lines +286 to +318

@gemini-code-assist gemini-code-assist Bot Oct 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Relying on the exact number of clock.currentTimeMillis() calls (numRefresh * 2 + 1) to coordinate thread synchronization is fragile. If the internal implementation of refreshCredentialsIfRequired() changes to call the clock more or fewer times, this test will become flaky or fail.

Instead, we can use a simple CountDownLatch(1) to pause the background thread in MockStsTransportFactory.create() until the main thread has finished its assertions. This completely decouples the synchronization from the internal clock calls and makes the test much more robust and maintainable.

You can then also revert the changes to createMockClock (and its signature) so it doesn't need to accept or interact with the latch.

    int numRefresh = 3;
    CountDownLatch refreshEntryLatch = new CountDownLatch(1);
    final ClientSideCredentialAccessBoundaryFactory factory =
        getClientSideCredentialAccessBoundaryFactory(RefreshType.ASYNC, refreshEntryLatch);

    // Call refreshCredentialsIfRequired() once to initialize the intermediate credentials. This
    // should make one request to the STS endpoint.
    factory.refreshCredentialsIfRequired();
    assertEquals(1, mockStsTransportFactory.transport.getRequestCount());

    try {
      // Subsequent calls to refreshCredentialsIfRequired() in an async mode should NOT
      // immediately call the STS endpoint. They should schedule an asynchronous refresh.
      for (int i = 0; i < numRefresh; i++) {
        factory.refreshCredentialsIfRequired();
      }

      // Verify that only the initial call resulted in an immediate STS request. The async refresh
      // is still pending.
      assertEquals(1, mockStsTransportFactory.transport.getRequestCount());
    } finally {
      refreshEntryLatch.countDown();
    }


// Introduce a small delay to allow the asynchronous refresh task to complete. This is
// necessary because the async task runs on a separate thread.
Expand Down
Loading