🧪 [testing improvement] Add unit tests for provider config accessors

[b3nw]

🎯 What: The testing gap addressed is the missing unit tests for get_full_provider_config and get_provider_ui_config functions in src/rotator_library/provider_config.py.
📊 Coverage: Test coverage is now complete for both known and unknown providers, verifying data merging and fallbacks.
✨ Result: Enhanced test coverage provides confidence against regressions and validates correct dictionary fallback behaviour.

---

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

[google-labs-jules]

👋 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]

Code Review

This pull request introduces unit tests for provider configuration accessors in tests/test_provider_config.py. The review feedback suggests enhancing the test suite by explicitly verifying the merge priority when combining data sources and refactoring multi-scenario tests using pytest parametrization for better isolation and readability.

[gemini-code-assist]

The test verifies that data from both sources is merged, but it doesn't verify the merge priority (i.e., that UI configuration overrides scraped data). Given that get_full_provider_config uses {**scraped, **ui_config}, it's important to ensure that if a key exists in both, the UI config value is the one preserved. It's also recommended to assert the entire dictionary to ensure no unexpected keys are present.

def test_get_full_provider_config_known_provider():
    """Test get_full_provider_config with a known provider, verifying merge priority."""
    mock_scraped_providers = {
        "test_provider": {
            "api_base": "https://api.test.com",
            "models": ["model-a"],
            "category": "scraped_category",
        }
    }
    mock_litellm_providers = {
        "test_provider": {
            "category": "test_category",
            "note": "Test note",
        }
    }
    with patch("rotator_library.provider_config.SCRAPED_PROVIDERS", mock_scraped_providers):
        with patch("rotator_library.provider_config.LITELLM_PROVIDERS", mock_litellm_providers):
            config = get_full_provider_config("test_provider")
            assert config == {
                "api_base": "https://api.test.com",
                "models": ["model-a"],
                "category": "test_category",
                "note": "Test note",
            }

[gemini-code-assist]

This test function covers two distinct scenarios. Using @pytest.mark.parametrize would make the test more concise, readable, and ensure that each case is executed independently.

Suggested change

	def test_get_full_provider_config_partial_data():
	"""Test get_full_provider_config when provider is only in scraped or only in UI config."""
	# Only in UI config
	mock_litellm_providers = {"test_ui_only": {"category": "ui_only_category"}}
	mock_scraped_providers = {}
	with patch("rotator_library.provider_config.SCRAPED_PROVIDERS", mock_scraped_providers):
	with patch("rotator_library.provider_config.LITELLM_PROVIDERS", mock_litellm_providers):
	config = get_full_provider_config("test_ui_only")
	assert config == {"category": "ui_only_category"}
	# Only in scraped config
	mock_litellm_providers = {}
	mock_scraped_providers = {"test_scraped_only": {"api_base": "https://test.com"}}
	with patch("rotator_library.provider_config.SCRAPED_PROVIDERS", mock_scraped_providers):
	with patch("rotator_library.provider_config.LITELLM_PROVIDERS", mock_litellm_providers):
	config = get_full_provider_config("test_scraped_only")
	assert config == {"api_base": "https://test.com", "category": "other"}
	@pytest.mark.parametrize("provider_key, mock_scraped, mock_ui, expected", [
	(
	"test_ui_only",
	{},
	{"test_ui_only": {"category": "ui_only_category"}},
	{"category": "ui_only_category"}
	),
	(
	"test_scraped_only",
	{"test_scraped_only": {"api_base": "https://test.com"}},
	{},
	{"api_base": "https://test.com", "category": "other"}
	),
	])
	def test_get_full_provider_config_partial_data(provider_key, mock_scraped, mock_ui, expected):
	"""Test get_full_provider_config when provider is only in scraped or only in UI config."""
	with patch("rotator_library.provider_config.SCRAPED_PROVIDERS", mock_scraped):
	with patch("rotator_library.provider_config.LITELLM_PROVIDERS", mock_ui):
	assert get_full_provider_config(provider_key) == expected

[b3nw]

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

1. [Gemini Code Assist] Merge Priority: Add a test case that explicitly verifies the merge priority when both SCRAPED_PROVIDERS and LITELLM_PROVIDERS contain the same key. The test should confirm which source "wins" for overlapping fields.
2. [Gemini Code Assist] Parametrization: Refactor the multi-scenario tests (test_get_full_provider_config_partial_data) using @pytest.mark.parametrize for better isolation and cleaner test reports.
3. [Boundary/Edge] Empty Data Scenarios: Verify behavior when dictionaries are present but empty (which you partially covered) and when they contain malformed entries (e.g., a provider key with a None value) to ensure the merge logic is resilient.

[google-labs-jules]

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

1. [Gemini Code Assist] Merge Priority: Add a test case that explicitly verifies the merge priority when both SCRAPED_PROVIDERS and LITELLM_PROVIDERS contain the same key. The test should confirm which source "wins" for overlapping fields.
2. [Gemini Code Assist] Parametrization: Refactor the multi-scenario tests (test_get_full_provider_config_partial_data) using @pytest.mark.parametrize for better isolation and cleaner test reports.
3. [Boundary/Edge] Empty Data Scenarios: Verify behavior when dictionaries are present but empty (which you partially covered) and when they contain malformed entries (e.g., a provider key with a None value) to ensure the merge logic is resilient.

I have implemented the requested changes: I added a test case to explicitly verify the merge priority when both SCRAPED_PROVIDERS and LITELLM_PROVIDERS contain the same key. I also refactored the multi-scenario tests using @pytest.mark.parametrize for better isolation and added a test to verify behavior when dictionaries contain malformed entries, such as a provider key with a None value. These edge cases required updating the logic in get_full_provider_config and get_provider_ui_config to be resilient to None values.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	0

Changes Reviewed

• src/rotator_library/provider_config.py - 2 functions modified to handle None values in provider dictionaries
• tests/test_provider_config.py - New tests covering known/unknown providers, merge priority, and edge cases

Assessment

The code changes are correct:

• get_provider_ui_config now explicitly returns {"category": "other"} when the provider key maps to None rather than returning None
• get_full_provider_config follows the same pattern for both SCRAPED_PROVIDERS and LITELLM_PROVIDERS
• Merge priority correctly gives UI config precedence over scraped data (test case at lines 89-94)

Tests are comprehensive and test the actual changed behavior.

Files Reviewed (2 files)

• src/rotator_library/provider_config.py - No issues
• tests/test_provider_config.py - No issues

---

_{Reviewed by minimax-m2.7 · 124,269 tokens}

[b3nw]

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

[google-labs-jules]

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

Acknowledged.