Skip to content

Fix test isolation for test_watsonx_gpt_oss_prompt_transformation#20474

Merged
ishaan-jaff merged 1 commit intomainfrom
litellm_fix_test_watsonx_gpt_oss_prompt_transformation_1017c3a
Feb 5, 2026
Merged

Fix test isolation for test_watsonx_gpt_oss_prompt_transformation#20474
ishaan-jaff merged 1 commit intomainfrom
litellm_fix_test_watsonx_gpt_oss_prompt_transformation_1017c3a

Conversation

@shin-bot-litellm
Copy link
Collaborator

Regression Fix

Failing Job: litellm_mapped_tests_llms
Caused By: Test isolation issue introduced in 1017c3a
Author: @ishaan-jaff

What Broke

Test test_watsonx_gpt_oss_prompt_transformation fails intermittently when running with parallel test execution (-n 16).

Error:

AssertionError: Prompt transformation failed - the template should have been applied to transform messages into the correct format for gpt-oss-120b.
assert None is not None

Original Commit LOC

The test originally:

  1. Cleared the litellm.known_tokenizer_config cache
  2. Mocked only async tokenizer functions (_aget_tokenizer_config, _aget_chat_template_file)

This Fix

The issue was a race condition with parallel tests:

  • Another test could populate the cache between clearing and usage
  • When cached, the sync code path is used instead of async
  • The sync path wasn't mocked, causing the template fetch to fail

Fix:

  1. Set cache directly instead of clearing it (avoids race condition)
  2. Also mock sync versions _get_tokenizer_config and _get_chat_template_file

This ensures the test is deterministic regardless of test execution order.

Set cached tokenizer config directly and mock both sync and async
tokenizer functions to avoid race conditions when running with
parallel test execution (-n 16).

The issue was that parallel tests could populate the
litellm.known_tokenizer_config cache between clearing it and
when the code checked it. This caused the sync code path to be
used instead of the async path, bypassing the mocked async functions.

Fix:
1. Set cache directly instead of clearing it
2. Also mock sync versions _get_tokenizer_config and _get_chat_template_file

This ensures the test is deterministic regardless of test execution order.
@vercel
Copy link

vercel bot commented Feb 5, 2026

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

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 5, 2026 6:30am

Request Review

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 5, 2026

Greptile Overview

Greptile Summary

This PR makes test_watsonx_gpt_oss_prompt_transformation deterministic under pytest-xdist by avoiding a cache-clear race and by mocking both async and sync HuggingFace template fetch helpers. The test now pre-populates litellm.known_tokenizer_config for openai/gpt-oss-120b and patches _get_* / _aget_* functions so prompt transformation always uses the mocked template regardless of which code path is hit.

One issue to address before merge: the test writes into the global litellm.known_tokenizer_config cache but never restores the prior value, which can leak mocked state into later tests within the same worker and reintroduce order dependence.

Confidence Score: 4/5

  • Mostly safe to merge, but test isolation needs one more fix to avoid leaking global cache state across tests.
  • Change is small and targeted (mocks sync+async paths to eliminate xdist flake). Main remaining risk is the new mutation of litellm.known_tokenizer_config without restoration, which can cause order-dependent failures in the same worker.
  • tests/test_litellm/llms/watsonx/test_watsonx.py

Important Files Changed

Filename Overview
tests/test_litellm/llms/watsonx/test_watsonx.py Fixes intermittent async test by pre-populating litellm.known_tokenizer_config and mocking sync/async HF template fetch; however, it now mutates global cache without restoring, which can leak into later tests.

Sequence Diagram

sequenceDiagram
  participant T as pytest (xdist worker)
  participant Test as test_watsonx_gpt_oss_prompt_transformation
  participant L as litellm.acompletion
  participant HF as huggingface_template_handler
  participant C as litellm.known_tokenizer_config
  participant W as AsyncHTTPHandler.post (mock)

  T->>Test: run test
  Test->>C: set C["openai/gpt-oss-120b"] = mock_tokenizer_config
  Note over Test,HF: Patch async and sync HF helpers
  Test->>L: acompletion(model=watsonx_text/openai/gpt-oss-120b, messages)
  L->>HF: resolve chat template
  alt cache hit / sync path
    HF->>C: read C[hf_model]
    HF-->>L: tokenizer_config with chat_template
  else async path
    HF->>HF: _aget_tokenizer_config (patched)
    HF->>HF: _aget_chat_template_file (patched -> failure)
    HF-->>L: tokenizer_config with chat_template
  end
  L->>W: POST completion (mock)
  W-->>L: mock_completion_response
  L-->>Test: response
  Test->>Test: assert request input contains template tags
Loading

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

Comment on lines +286 to +291
# Set cached tokenizer config directly to avoid race conditions with parallel tests.
# When running with pytest-xdist (-n 16), another test might populate the cache between
# clearing it and the actual usage. By setting the cache directly, we ensure the correct
# template is always used regardless of test execution order.
hf_model = "openai/gpt-oss-120b"
if hf_model in litellm.known_tokenizer_config:
del litellm.known_tokenizer_config[hf_model]
litellm.known_tokenizer_config[hf_model] = mock_tokenizer_config
Copy link
Contributor

Choose a reason for hiding this comment

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

Global cache not restored

This test now writes to the module-level litellm.known_tokenizer_config (litellm.known_tokenizer_config[hf_model] = ...) but never restores the previous value. That makes the test order-dependent: subsequent tests in the same worker will see the mocked tokenizer config and may skip their intended code paths. Consider snapshotting the previous entry (or absence) and restoring it in a try/finally (or using a fixture) so the global cache is returned to its prior state when the test completes.

Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_litellm/llms/watsonx/test_watsonx.py
Line: 286:291

Comment:
**Global cache not restored**

This test now writes to the module-level `litellm.known_tokenizer_config` (`litellm.known_tokenizer_config[hf_model] = ...`) but never restores the previous value. That makes the test order-dependent: subsequent tests in the same worker will see the mocked tokenizer config and may skip their intended code paths. Consider snapshotting the previous entry (or absence) and restoring it in a `try/finally` (or using a fixture) so the global cache is returned to its prior state when the test completes.

How can I resolve this? If you propose a fix, please make it concise.

@ishaan-jaff ishaan-jaff merged commit 1d92968 into main Feb 5, 2026
59 of 66 checks passed
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.

3 participants