refactor(tests): address code review feedback

- Remove redundant @patch.object decorators (inner context manager suffices)
- Remove try/except blocks that could hide test failures
- Tests now fail fast if mocking is insufficient
This commit is contained in:
Robert Hyman
2025-11-29 00:55:41 -05:00
parent 1f8b58d607
commit 0c3e63c0c7

View File

@@ -68,8 +68,7 @@ class TestStoreParameterHandling(unittest.TestCase):
self.openrouter_provider = MockOpenRouterProvider("test-key") self.openrouter_provider = MockOpenRouterProvider("test-key")
self.openai_provider = MockOpenAIProvider("test-key") self.openai_provider = MockOpenAIProvider("test-key")
@patch.object(OpenAICompatibleProvider, "client", new_callable=lambda: property(lambda self: Mock())) def test_openrouter_responses_omits_store_parameter(self):
def test_openrouter_responses_omits_store_parameter(self, mock_client):
"""Test that OpenRouter provider omits store parameter from responses endpoint. """Test that OpenRouter provider omits store parameter from responses endpoint.
**Feature: openrouter-store-parameter-fix, Property 1: OpenRouter requests omit store parameter** **Feature: openrouter-store-parameter-fix, Property 1: OpenRouter requests omit store parameter**
@@ -98,20 +97,16 @@ class TestStoreParameterHandling(unittest.TestCase):
provider = MockOpenRouterProvider("test-key") provider = MockOpenRouterProvider("test-key")
# Call the method that builds completion_params # Call the method that builds completion_params
try:
provider._generate_with_responses_endpoint( provider._generate_with_responses_endpoint(
model_name="openai/gpt-5-pro", model_name="openai/gpt-5-pro",
messages=[{"role": "user", "content": "test"}], messages=[{"role": "user", "content": "test"}],
temperature=0.7, temperature=0.7,
) )
except Exception:
pass # We only care about the captured params
# Verify store parameter is NOT in the request # Verify store parameter is NOT in the request
self.assertNotIn("store", captured_params, "OpenRouter requests should NOT include 'store' parameter") self.assertNotIn("store", captured_params, "OpenRouter requests should NOT include 'store' parameter")
@patch.object(OpenAICompatibleProvider, "client", new_callable=lambda: property(lambda self: Mock())) def test_openai_responses_includes_store_parameter(self):
def test_openai_responses_includes_store_parameter(self, mock_client):
"""Test that direct OpenAI provider includes store parameter in responses endpoint. """Test that direct OpenAI provider includes store parameter in responses endpoint.
**Feature: openrouter-store-parameter-fix, Property 2: Direct OpenAI requests include store parameter** **Feature: openrouter-store-parameter-fix, Property 2: Direct OpenAI requests include store parameter**
@@ -140,14 +135,11 @@ class TestStoreParameterHandling(unittest.TestCase):
provider = MockOpenAIProvider("test-key") provider = MockOpenAIProvider("test-key")
# Call the method that builds completion_params # Call the method that builds completion_params
try:
provider._generate_with_responses_endpoint( provider._generate_with_responses_endpoint(
model_name="gpt-5-pro", model_name="gpt-5-pro",
messages=[{"role": "user", "content": "test"}], messages=[{"role": "user", "content": "test"}],
temperature=0.7, temperature=0.7,
) )
except Exception:
pass # We only care about the captured params
# Verify store parameter IS in the request with value True # Verify store parameter IS in the request with value True
self.assertIn("store", captured_params, "OpenAI requests should include 'store' parameter") self.assertIn("store", captured_params, "OpenAI requests should include 'store' parameter")