Skip to content

🧪 test(core): Add tests for get_error_tracker - #38

Closed
b3nw wants to merge 13 commits into
devfrom
test-error-tracker-14167267346632025388
Closed

🧪 test(core): Add tests for get_error_tracker#38
b3nw wants to merge 13 commits into
devfrom
test-error-tracker-14167267346632025388

Conversation

@b3nw

@b3nw b3nw commented Apr 25, 2026

Copy link
Copy Markdown
Owner

🎯 What: The testing gap addressed
The get_error_tracker function in src/rotator_library/error_tracker.py lacked test coverage, specifically regarding its singleton behavior and thread-safe double-checked locking lazy initialization.

📊 Coverage: What scenarios are now tested

  • Verification that get_error_tracker returns an instance of ErrorTracker.
  • Verification of the singleton pattern: multiple sequential calls return the exact same instance.
  • Verification of thread-safe initialization: using threading.Barrier to ensure multiple concurrent threads all receive the same initialized instance without race conditions.

Result: The improvement in test coverage
The thread-safe lazy initialization and singleton logic are now fully covered and guaranteed to prevent race conditions during initialization.


PR created automatically by Jules for task 14167267346632025388 started by @b3nw

b3nw and others added 10 commits April 24, 2026 19:40
…ardization, and utilities

Core infrastructure improvements:
- Smart 'latest' model alias resolution with cost-based tiebreaking
- Standardized error responses with proper HTTP status codes and error.code field
- ProxyExhaustionError for structured credential exhaustion reporting
- TerminalRequestError for non-rotatable errors (404, model not found)
- Per-provider retry count override via MAX_RETRIES_{PROVIDER} env var
- Retry 429 rate_limit errors with backoff instead of rotating
- Cached token pricing in streaming cost calculation
- Split quota stats into current_period and global/lifetime views
- Log rotation for proxy.log and proxy_debug.log (RotatingFileHandler)
- Include latest virtual models in /v1/models endpoint
- Resolve singleton cache pollution for dynamic providers
- Fork-specific README and .gitignore updates
…ased model filtering, and enhanced X-Initiator heuristic
Test suite designed to catch breakage from branch re-organization without
sending queries to real LLM providers. Covers all critical integration
points that previously broke silently during deployment.

Coverage:
- Anthropic↔OpenAI format translation & streaming
- Error classification (determines retry/rotation behavior)
- Request sanitization (prevents 400s from invalid params)
- Provider-specific request transforms
- Model alias & latest registry parsing
- Usage tracking (windows, quota groups, custom caps)
- Credential discovery, deduplication, env:// URI
- Provider plugin registration & singleton pattern
- Proxy endpoint routing & auth

All tests use synthetic credentials and mocked HTTP.
Runs in ~2.3s. Zero API cost.
…flow

Replaces the old manifest-driven multi-branch replay system with a
simpler linear commit stack. Changes are made via fixup!/autosquash.
Upstream syncs are a single git rebase.

Includes:
- AGENTS.md: entry point for all AI coding agents
- .agent/rules/claude.md: Claude-specific SSH/deployment notes
- .agent/rules/llm-proxy.md: container layout and deployment pipeline
- .agent/skills/upstream-sync/SKILL.md: sync workflow reference
Custom provider for Google Vertex AI Express Mode API keys that uses
x-goog-api-key header authentication against the Vertex AI
OpenAI-compatible endpoint. Supports non-streaming and streaming
chat completions with automatic model discovery.

Models are prefixed as vertex/ (e.g. vertex/gemini-3.1-flash-lite-preview).
Env vars: VERTEX_PROJECT, VERTEX_LOCATION, VERTEX_API_KEY_N
Adds missing tests to verify the singleton pattern and thread-safe lazy initialization of `get_error_tracker` in `error_tracker.py`.
@google-labs-jules

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

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

Copy link
Copy Markdown

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 introduces a new test suite for the ErrorTracker class, covering singleton behavior and thread-safe initialization. Feedback suggests replacing the brittle manual module loading with standard imports to avoid path dependency and namespace pollution. Additionally, it is recommended to add a timeout to the barrier synchronization in the thread-safety test to prevent the test suite from hanging indefinitely.

Comment thread tests/test_error_tracker.py Outdated
Comment on lines +7 to +10
spec = importlib.util.spec_from_file_location("error_tracker", "src/rotator_library/error_tracker.py")
error_tracker_module = importlib.util.module_from_spec(spec)
sys.modules["error_tracker"] = error_tracker_module
spec.loader.exec_module(error_tracker_module)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The manual module loading using importlib.util with a hardcoded relative path is brittle and pollutes the global sys.modules namespace. This approach makes the tests dependent on the current working directory and can cause naming collisions. Standard imports are preferred unless there is a specific need to bypass package-level initialization.

Comment thread tests/test_error_tracker.py Outdated
results = []

def worker():
barrier.wait()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Calling barrier.wait() without a timeout can cause the test suite to hang indefinitely if a thread fails to reach the barrier. Providing a timeout ensures the test fails predictably.

Suggested change
barrier.wait()
barrier.wait(timeout=5)

Adds missing tests to verify the singleton pattern and thread-safe lazy initialization of `get_error_tracker` in `error_tracker.py`.
@b3nw

b3nw commented Apr 26, 2026

Copy link
Copy Markdown
Owner Author

[Gemini-CLI Review Skill] @jules Reviewing this PR against the checklist and Gemini Code Assist feedback:

  1. [Gemini Code Assist] Module Loading: Please replace the manual importlib.util loading with standard imports. Manual loading can lead to hardcoded path dependencies and namespace issues.
  2. [Gemini Code Assist] Hang Prevention: In test_get_error_tracker_thread_safe_initialization, add a timeout to the barrier.wait() and t.join() calls. This prevents the entire test suite from hanging indefinitely if a deadlock occurs in the initialization logic.
  3. [Review Checklist] Teardown: Ensure that the reset_error_tracker fixture is robust enough to clear any other internal state the ErrorTracker might have (like recorded errors) to keep tests fully isolated.

@google-labs-jules

Copy link
Copy Markdown

[Gemini-CLI Review Skill] @jules Reviewing this PR against the checklist and Gemini Code Assist feedback:

  1. [Gemini Code Assist] Module Loading: Please replace the manual importlib.util loading with standard imports. Manual loading can lead to hardcoded path dependencies and namespace issues.
  2. [Gemini Code Assist] Hang Prevention: In test_get_error_tracker_thread_safe_initialization, add a timeout to the barrier.wait() and t.join() calls. This prevents the entire test suite from hanging indefinitely if a deadlock occurs in the initialization logic.
  3. [Review Checklist] Teardown: Ensure that the reset_error_tracker fixture is robust enough to clear any other internal state the ErrorTracker might have (like recorded errors) to keep tests fully isolated.

I have addressed all the feedback points:

  1. Replaced importlib.util with standard import, using sys.modules to mock missing dependencies for the isolated test environment.
  2. Added a 5.0 second timeout to both barrier.wait() and t.join() in test_get_error_tracker_thread_safe_initialization to prevent any potential test hangs.
  3. Updated the reset_error_tracker fixture to call _error_tracker.clear() to completely remove any recorded errors and guarantee test isolation.

- Replace manual module loading with standard imports
- Add 5.0s timeout to `barrier.wait()` and `t.join()` to prevent test hangs
- Add `.clear()` to the `reset_error_tracker` fixture to correctly isolate test state
@b3nw
b3nw marked this pull request as ready for review April 26, 2026 03:23
- Replace manual module loading with standard imports
- Add 5.0s timeout to `barrier.wait()` and `t.join()` to prevent test hangs
- Add `.clear()` to the `reset_error_tracker` fixture to correctly isolate test state
@b3nw

b3nw commented Apr 26, 2026

Copy link
Copy Markdown
Owner Author

[Gemini-CLI Review Skill] Consolidated into the linear commit stack on dev.

@b3nw b3nw closed this Apr 26, 2026
@google-labs-jules

Copy link
Copy Markdown

[Gemini-CLI Review Skill] Consolidated into the linear commit stack on dev.

Acknowledged.

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