Skip to content
This repository was archived by the owner on Sep 27, 2026. It is now read-only.

🧪 Add test for safe_write_json failure buffering fallback - #47

Closed
b3nw wants to merge 12 commits into
devfrom
test-resilient-io-buffer-15860739152021086132
Closed

b3nw wants to merge 12 commits into
devfrom
test-resilient-io-buffer-15860739152021086132

Conversation

@b3nw

@b3nw b3nw commented Apr 25, 2026

Copy link
Copy Markdown
Owner

🎯 What: The testing gap addressed was missing tests for safe_write_json error fallback handling in resilient_io.py.
📊 Coverage: Now tests the scenario where tempfile.mkstemp fails with an OSError, ensuring that the buffer_on_failure fallback mechanism registers the pending write with BufferedWriteRegistry.
✨ Result: Enhanced test coverage and reliability around the resilient IO utility functions, catching regressions in file write buffering.


PR created automatically by Jules for task 15860739152021086132 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
@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 case, test_safe_write_json_oserror_buffering, to verify that safe_write_json correctly handles OSError by buffering the write operation. The review feedback suggests cleaning up unused imports and improving the test's robustness by asserting the specific arguments passed to the registry's register_pending method.

Comment thread tests/utils/test_resilient_io.py Outdated
Comment on lines +2 to +6
import os
import json
import logging
from unittest.mock import patch, mock_open, MagicMock
from pathlib import Path

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

Several imports are unused in this test file (os, json, mock_open, Path). Removing them improves code cleanliness. Additionally, ANY is added to support more specific assertions in the test logic.

Suggested change
import os
import json
import logging
from unittest.mock import patch, mock_open, MagicMock
from pathlib import Path
import logging
from unittest.mock import ANY, MagicMock, patch

Comment thread tests/utils/test_resilient_io.py Outdated

assert result is False
mock_mkstemp.assert_called_once()
mock_registry_instance.register_pending.assert_called_once()

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

It is recommended to verify the arguments passed to register_pending to ensure the correct data and configuration are being buffered. Since the serializer is a local function, ANY is used to match it.

Suggested change
mock_registry_instance.register_pending.assert_called_once()
mock_registry_instance.register_pending.assert_called_once_with(
path, data, ANY, {"secure_permissions": False}
)

@b3nw

b3nw commented Apr 25, 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] Robust Assertions: Please update the register_pending assertion to verify the specific arguments (path, data, serializer, and options). This ensures we aren't just calling the function, but calling it with the correct state.
  2. [Gemini Code Assist] Cleanup: Remove unused imports such as os, json, and mock_open to keep the test file clean.
  3. [Boundary/Edge] Error Type Verification: Add a test case that mocks a different error type (e.g., PermissionError) to ensure the buffering mechanism is generic to the exceptions caught by safe_write_json.
  4. [Refactoring] Formatting: Ensure the test uses the project's standard 4-space indentation (the diff shows correct indentation, but double-check against surrounding files).

@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] Robust Assertions: Please update the register_pending assertion to verify the specific arguments (path, data, serializer, and options). This ensures we aren't just calling the function, but calling it with the correct state.
  2. [Gemini Code Assist] Cleanup: Remove unused imports such as os, json, and mock_open to keep the test file clean.
  3. [Boundary/Edge] Error Type Verification: Add a test case that mocks a different error type (e.g., PermissionError) to ensure the buffering mechanism is generic to the exceptions caught by safe_write_json.
  4. [Refactoring] Formatting: Ensure the test uses the project's standard 4-space indentation (the diff shows correct indentation, but double-check against surrounding files).

I have updated the assertions to verify specific arguments on register_pending using assert_called_once_with. I've also removed the unused imports (os, json, mock_open) to clean up the test file, and added an additional test case test_safe_write_json_permissionerror_buffering that mocks a PermissionError. Indentation continues to be standard 4 spaces.

@b3nw
b3nw marked this pull request as ready for review April 26, 2026 03:23
@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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant