Skip to content

fix(test): add missing mock for get_vertex_ai_credentials#21274

Closed
jquinter wants to merge 1 commit intomainfrom
fix/vertex-ai-rerank-optional-params-test
Closed

fix(test): add missing mock for get_vertex_ai_credentials#21274
jquinter wants to merge 1 commit intomainfrom
fix/vertex-ai-rerank-optional-params-test

Conversation

@jquinter
Copy link
Contributor

Summary

Fixes test failure by adding missing mock for get_vertex_ai_credentials.

Problem

Test test_validate_environment_with_optional_params was failing with:

Exception: Unable to load vertex credentials from environment. 
Got=path/to/credentials.json

The test passes "path/to/credentials.json" as fake credentials, but the code tries to load this file for real, which:

  1. Doesn't exist as a file
  2. Isn't valid JSON
  3. Causes an exception

Root Cause

The test test_validate_environment (line 92) correctly mocks get_vertex_ai_credentials:

with patch.object(self.config, 'get_vertex_ai_credentials', return_value=None):

But test_validate_environment_with_optional_params was missing this mock, causing it to try loading the fake credential file for real.

Solution

Added the same mock pattern to test_validate_environment_with_optional_params:

with patch.object(self.config, 'get_vertex_ai_credentials', return_value=None):
    headers = self.config.validate_environment(...)

Testing

pytest tests/.../test_vertex_ai_rerank_transformation.py::TestVertexAIRerankTransform::test_validate_environment_with_optional_params -v
======================== 1 passed in 0.18s ========================

Related

This test failure was reported on PR #21217, but NOT caused by PR #21217 (which only modifies Anthropic structured output tests). This is a missing mock in the test itself.

🤖 Generated with Claude Code

@vercel
Copy link

vercel bot commented Feb 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 15, 2026 10:32pm

Request Review

Add missing mock for get_vertex_ai_credentials in
test_validate_environment_with_optional_params to prevent it from
trying to load the fake credential file "path/to/credentials.json".

The test was failing with:
"Exception: Unable to load vertex credentials from environment.
Got=path/to/credentials.json"

The test test_validate_environment already has this mock (line 92),
but test_validate_environment_with_optional_params was missing it.

Adding the same mock pattern:
patch.object(self.config, 'get_vertex_ai_credentials', return_value=None)

Test now passes ✅

Related: test was failing on PR #21217, but NOT caused by PR #21217
(which only modifies test_anthropic_structured_output.py). This is a
missing mock in the test.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@jquinter jquinter force-pushed the fix/vertex-ai-rerank-optional-params-test branch from a4cbb83 to 00bb393 Compare February 15, 2026 22:31
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 15, 2026

Greptile Summary

This PR adds a patch.object mock for get_vertex_ai_credentials to test_validate_environment_with_optional_params, intended to prevent the test from trying to load a fake credential file path.

However, validate_environment in transformation.py calls self.safe_get_vertex_ai_credentials() (the non-mutating variant), not self.get_vertex_ai_credentials(). The added mock targets the wrong method and has no practical effect. The test passes because _ensure_access_token is already mocked via the @patch decorator on the test method, which intercepts the call before any real credential loading.

  • The mock for get_vertex_ai_credentials should instead target safe_get_vertex_ai_credentials if the intent is to prevent credential string resolution
  • The sibling test test_validate_environment_preserves_optional_params_for_get_complete_url (line 476) has the same pattern (fake credentials, no mock for the safe variant) and may have the same latent issue

Confidence Score: 3/5

  • Low risk test-only change, but the mock targets the wrong method and has no effect on test behavior
  • The change is a test-only fix with no production code impact, so it's safe to merge. However, the mock targets get_vertex_ai_credentials while the code under test calls safe_get_vertex_ai_credentials, meaning the mock is a no-op. The test passes due to the existing _ensure_access_token mock, not this new one. Score of 3 reflects that while harmless, the fix doesn't address its stated purpose.
  • tests/test_litellm/llms/vertex_ai/rerank/test_vertex_ai_rerank_transformation.py — mock targets get_vertex_ai_credentials instead of safe_get_vertex_ai_credentials

Important Files Changed

Filename Overview
tests/test_litellm/llms/vertex_ai/rerank/test_vertex_ai_rerank_transformation.py Adds a mock for get_vertex_ai_credentials in test_validate_environment_with_optional_params, but validate_environment actually calls safe_get_vertex_ai_credentials — the mock targets the wrong method and has no effect. The test passes because _ensure_access_token is already mocked via the @patch decorator.

Sequence Diagram

sequenceDiagram
    participant Test as Test Method
    participant Config as VertexAIRerankConfig
    participant SafeGet as safe_get_vertex_ai_credentials
    participant GetCreds as get_vertex_ai_credentials
    participant EnsureToken as _ensure_access_token

    Note over GetCreds: MOCKED but never called
    Note over EnsureToken: MOCKED via decorator

    Test->>Config: validate_environment(optional_params)
    Config->>SafeGet: safe_get_vertex_ai_credentials(params)
    SafeGet-->>Config: credential string from optional_params
    Config->>EnsureToken: called with credential string
    EnsureToken-->>Config: mocked return value
    Config-->>Test: headers dict
Loading

Last reviewed commit: a4cbb83

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

assert headers == expected_headers

# Mock get_vertex_ai_credentials to prevent it from trying to load the fake credential file
with patch.object(self.config, 'get_vertex_ai_credentials', return_value=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Mock targets wrong method

validate_environment (in transformation.py:80) calls self.safe_get_vertex_ai_credentials(litellm_params), not self.get_vertex_ai_credentials(...). Since safe_get_vertex_ai_credentials is the method actually invoked, this patch.object for get_vertex_ai_credentials has no effect — it mocks a method that is never called in this code path.

The test already works because _ensure_access_token is mocked via the @patch decorator on the test method, which intercepts the call before any real credential loading occurs.

If the intent is to prevent safe_get_vertex_ai_credentials from returning the fake credential string, the mock should target that method instead:

Suggested change
with patch.object(self.config, 'get_vertex_ai_credentials', return_value=None):
with patch.object(self.config, 'safe_get_vertex_ai_credentials', return_value=None):

Note: the same pattern may need to be applied to test_validate_environment_preserves_optional_params_for_get_complete_url (line 476) which also passes fake credentials without mocking.

@jquinter
Copy link
Contributor Author

Closing this PR. The test failures were due to environment variable pollution from other tests, which was already fixed by PR #21268 that adds setup_method/teardown_method to clean environment variables.

As Greptile correctly identified, the mock added in this PR is ineffective - it mocks get_vertex_ai_credentials but the code actually calls safe_get_vertex_ai_credentials. The test passes because _ensure_access_token is already mocked via the @patch decorator, preventing any credential loading.

The tests now pass on main branch in isolation, confirming the environment cleanup in #21268 resolved the issue.

@jquinter jquinter closed this Feb 15, 2026
@jquinter jquinter deleted the fix/vertex-ai-rerank-optional-params-test branch February 15, 2026 22:35
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.

1 participant