Skip to content

fix: resolve failing getApplicationDefault unit tests in gcloud env#3163

Merged
lahirumaramba merged 1 commit into
v14from
lm-adc-test
May 27, 2026
Merged

fix: resolve failing getApplicationDefault unit tests in gcloud env#3163
lahirumaramba merged 1 commit into
v14from
lm-adc-test

Conversation

@lahirumaramba
Copy link
Copy Markdown
Member

This PR resolves the unit test failures in getApplicationDefault and isApplicationDefault that occur on local machines where gcloud application default credentials exist. It also fixes a potential environment pollution issue in the test suite and cleans up unused ESLint directives.

Root Cause of Failures

In a prior migration to google-auth-library, getApplicationDefault() was refactored to return an instance of ApplicationDefaultCredential (which dynamically delegates credential resolution under the hood) instead of manually parsing credential files and returning specific wrappers like RefreshTokenCredential.

However, two unit tests still asserted that the return value of getApplicationDefault() was directly an instanceof RefreshTokenCredential. On environments without local gcloud credentials (like CI), these tests were silently skipped. In environments where the developer has authenticated via gcloud, these tests executed and failed.

Changes Implemented

  1. Modernized Spec Assertions:

    • Updated the spec to check that getApplicationDefault() returns ApplicationDefaultCredential.
    • Resolved the underlying authenticating client using (c as any).googleAuth.getClient() and verified it resolves to an instance of UserRefreshClient (from google-auth-library), preserving the exact test coverage and intent.
    • Injected a mock process.env.GOOGLE_CLOUD_PROJECT inside these asynchronous test cases to prevent google-auth-library from hanging during GCE metadata service auto-discovery on local environments.
  2. Isolated test environment variables (process.env):

    • Fixed the beforeEach and afterEach environment setup/teardown in credential-internal.spec.ts to shallow-copy and properly restore environment variables, preventing modified environment variables from polluting other test suites.
  3. ESLint directive cleanups:

    • Removed unused eslint-disable-next-line @typescript-eslint/no-var-requires comments in crypto-signer files to resolve linting warnings.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request cleans up ESLint disable comments for node:crypto imports, improves environment variable isolation in unit tests by properly copying and restoring process.env, and updates credential tests to handle asynchronous client retrieval and assert the correct types for ApplicationDefaultCredential and UserRefreshClient. I have no feedback to provide as there are no active review comments.

Copy link
Copy Markdown
Collaborator

@jonathanedey jonathanedey left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

@lahirumaramba lahirumaramba merged commit 1b0a0d0 into v14 May 27, 2026
14 checks passed
@lahirumaramba lahirumaramba deleted the lm-adc-test branch May 27, 2026 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants