fix test failing due to concurrent azcli token invocation

[tanmaya-panda1]

This pull request introduces a new test-only credential class to optimize token acquisition for test environments and updates test base classes to use this credential. The main improvement is preventing multiple concurrent requests to the Azure CLI for access tokens, which can cause failures in parallel test executions. The changes are focused on reliability and efficiency in test setup.

Credential optimization:

• Added CachingTokenCredential, a singleton class that acquires an Azure CLI token once and reuses it for all test requests, reducing redundant subprocess invocations and improving test stability (CachingTokenCredential.java).

Test base refactoring:

• Updated both Java (IngestV2JavaTestBase) and Kotlin (IngestV2TestBase) test base classes to use the shared CachingTokenCredential instead of creating a new Azure CLI credential instance for each test, ensuring only one token fetch per test run. [1] [2]
• Modified test client creation logic to use ConnectionStringBuilder.createWithTokenCredential with the shared credential, replacing the previous Azure CLI-based approach. [1] [2]

Cleanup:

• Removed unused imports related to AzureCliCredentialBuilder from test files. [1] [2]

[Copilot]

Pull request overview

This pull request introduces a CachingTokenCredential class to optimize test execution by preventing redundant and concurrent Azure CLI subprocess invocations during parallel test runs. The implementation wraps AzureCliCredential with a per-scope token cache and updates both Java and Kotlin test base classes to use a shared singleton instance instead of creating individual credentials per test.

Changes:

• Added CachingTokenCredential class with scope-based token caching and thread-safe double-checked locking pattern
• Updated IngestV2JavaTestBase and IngestV2TestBase to use CachingTokenCredential.INSTANCE singleton
• Removed unused AzureCliCredentialBuilder imports from test base classes

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
ingest-v2/src/test/java/com/microsoft/azure/kusto/ingest/v2/CachingTokenCredential.java	New credential wrapper implementing per-scope token caching with double-checked locking to prevent concurrent subprocess calls
ingest-v2/src/test/java/com/microsoft/azure/kusto/ingest/v2/IngestV2JavaTestBase.java	Updated to use shared CachingTokenCredential.INSTANCE instead of creating new AzureCliCredential instances
ingest-v2/src/test/kotlin/com/microsoft/azure/kusto/ingest/v2/IngestV2TestBase.kt	Updated to use shared CachingTokenCredential.INSTANCE instead of creating new AzureCliCredential instances

Comments suppressed due to low confidence (4)

ingest-v2/src/test/java/com/microsoft/azure/kusto/ingest/v2/CachingTokenCredential.java:49

• The scope key generation using stream().collect(Collectors.joining(",")) relies on the iteration order of the scopes collection. If TokenRequestContext.getScopes() returns a collection with non-deterministic ordering (e.g., a HashSet), the same set of scopes could produce different cache keys like "scope1,scope2" vs "scope2,scope1", leading to cache misses and redundant token fetches. Consider sorting the scopes before joining them to ensure consistent cache keys, or use a TreeSet to join sorted scopes.

        String scopeKey = request.getScopes().stream().collect(Collectors.joining(","));

ingest-v2/src/test/java/com/microsoft/azure/kusto/ingest/v2/CachingTokenCredential.java:68

• Calling .block() on a Mono inside Mono.fromCallable() defeats the purpose of reactive programming and can cause thread starvation or deadlocks if executed on a non-blocking scheduler (e.g., Reactor's parallel or single scheduler). This blocking call will tie up a thread waiting for the token, which could be problematic in high-concurrency scenarios. Consider using reactive composition like delegate.getToken(request).map(...) instead, though this may require restructuring the double-checked locking pattern to be fully reactive. If blocking is intentional for test simplicity, add a comment explaining this design choice.

                AccessToken newToken = delegate.getToken(request).block();

ingest-v2/src/test/java/com/microsoft/azure/kusto/ingest/v2/CachingTokenCredential.java:34

• The static INSTANCE field eagerly initializes AzureCliCredential at class load time, which will fail immediately if Azure CLI is not installed or configured, even if the class is loaded but never used (e.g., when running unit tests that don't need real credentials). Consider lazy initialization using a holder pattern or synchronized method to defer credential creation until first use, allowing unit tests to run without Azure CLI dependencies.

    public static final TokenCredential INSTANCE =
            new CachingTokenCredential(new AzureCliCredentialBuilder().build());

ingest-v2/src/test/java/com/microsoft/azure/kusto/ingest/v2/CachingTokenCredential.java:79

• The CachingTokenCredential class implements complex concurrency logic (double-checked locking, token refresh timing, scope-based caching) but lacks unit tests. Given that similar utility classes in the codebase have dedicated test files (e.g., ConnectionStringBuilderTest.java, AadAuthenticationHelperTest.java), and considering the critical nature of this class for test reliability, add unit tests covering: concurrent token requests for the same scope, different scopes, token expiry and refresh logic, and thread safety under high concurrency.

public class CachingTokenCredential implements TokenCredential {

    /** Process-wide shared instance wrapping {@code AzureCliCredential}, used by all E2E tests. */
    public static final TokenCredential INSTANCE =
            new CachingTokenCredential(new AzureCliCredentialBuilder().build());

    private static final long TOKEN_REFRESH_MARGIN_MINUTES = 5L;

    private final TokenCredential delegate;
    private final ReentrantLock lock = new ReentrantLock();
    // Per-scope cache: scope string → AccessToken
    private final Map<String, AccessToken> tokenCache = new HashMap<>();

    public CachingTokenCredential(TokenCredential delegate) {
        this.delegate = delegate;
    }

    @Override
    public Mono<AccessToken> getToken(TokenRequestContext request) {
        String scopeKey = request.getScopes().stream().collect(Collectors.joining(","));

        // Fast path: return cached token without acquiring lock if not near expiry
        AccessToken current = tokenCache.get(scopeKey);
        if (current != null && current.getExpiresAt().isAfter(
                OffsetDateTime.now().plusMinutes(TOKEN_REFRESH_MARGIN_MINUTES))) {
            return Mono.just(current);
        }

        // Slow path: acquire lock, refresh if still needed (double-checked locking)
        return Mono.fromCallable(() -> {
            lock.lock();
            try {
                AccessToken recheck = tokenCache.get(scopeKey);
                if (recheck != null && recheck.getExpiresAt().isAfter(
                        OffsetDateTime.now().plusMinutes(TOKEN_REFRESH_MARGIN_MINUTES))) {
                    return recheck;
                }
                // Only one thread reaches here per scope at a time
                AccessToken newToken = delegate.getToken(request).block();
                if (newToken == null) {
                    throw new IllegalStateException("TokenCredential.getToken() returned null");
                }
                tokenCache.put(scopeKey, newToken);
                return newToken;
            } finally {
                lock.unlock();
            }
        });
    }
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Test Results

530 tests  ±0   521 ✅ ±0   7m 59s ⏱️ -26s
 31 suites ±0     9 💤 ±0 
 31 files   ±0     0 ❌ ±0 

Results for commit 91d73f0. ± Comparison against base commit bad4866.

[github-actions]

Test Results

530 tests  ±0   521 ✅ ±0   8m 9s ⏱️ -16s
 31 suites ±0     9 💤 ±0 
 31 files   ±0     0 ❌ ±0 

Results for commit 9a5f82b. ± Comparison against base commit bad4866.

♻️ This comment has been updated with latest results.

[ag-ramachandran]

Do we need a seperate class, why not create AzureCliCredential in the IngestV2Base, and initialize and cache it once across tests.

[tanmaya-panda1]

Since we have IngestV2TestBase.kt as well as IngestV2TestBase.java, so it will need 2 repetitive initialization

[ag-ramachandran]

This token provider here can be AzureCliTokenProvider perhaps , initialized once