From dd2efac3ae274edfae7040c7ab510305ddea1b6b Mon Sep 17 00:00:00 2001 From: "Paul Gauthier (aider)" Date: Tue, 18 Mar 2025 17:33:52 -0700 Subject: [PATCH] test: verify Model.set_ methods are called appropriately in tests --- tests/basic/test_main.py | 82 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 4 deletions(-) diff --git a/tests/basic/test_main.py b/tests/basic/test_main.py index 5d7a8ebc9..a0b10e394 100644 --- a/tests/basic/test_main.py +++ b/tests/basic/test_main.py @@ -688,7 +688,8 @@ class TestMain(TestCase): # Test that appropriate warnings are shown based on accepts_settings configuration with GitTemporaryDirectory(): # Test model that accepts the thinking_tokens setting - with patch("aider.io.InputOutput.tool_warning") as mock_warning: + with patch("aider.io.InputOutput.tool_warning") as mock_warning, \ + patch("aider.models.Model.set_thinking_tokens") as mock_set_thinking: main( [ "--model", @@ -704,9 +705,12 @@ class TestMain(TestCase): # No warning should be shown as this model accepts thinking_tokens for call in mock_warning.call_args_list: self.assertNotIn("thinking_tokens", call[0][0]) + # Method should be called + mock_set_thinking.assert_called_once_with("1000") # Test model that doesn't have accepts_settings for thinking_tokens - with patch("aider.io.InputOutput.tool_warning") as mock_warning: + with patch("aider.io.InputOutput.tool_warning") as mock_warning, \ + patch("aider.models.Model.set_thinking_tokens") as mock_set_thinking: main( ["--model", "gpt-4o", "--thinking-tokens", "1000", "--yes", "--exit"], input=DummyInput(), @@ -718,9 +722,12 @@ class TestMain(TestCase): if "thinking_tokens" in call[0][0]: warning_shown = True self.assertTrue(warning_shown) + # Method should still be called by default + mock_set_thinking.assert_called_once_with("1000") # Test model that accepts the reasoning_effort setting - with patch("aider.io.InputOutput.tool_warning") as mock_warning: + with patch("aider.io.InputOutput.tool_warning") as mock_warning, \ + patch("aider.models.Model.set_reasoning_effort") as mock_set_reasoning: main( ["--model", "o1", "--reasoning-effort", "3", "--yes", "--exit"], input=DummyInput(), @@ -729,9 +736,12 @@ class TestMain(TestCase): # No warning should be shown as this model accepts reasoning_effort for call in mock_warning.call_args_list: self.assertNotIn("reasoning_effort", call[0][0]) + # Method should be called + mock_set_reasoning.assert_called_once_with("3") # Test model that doesn't have accepts_settings for reasoning_effort - with patch("aider.io.InputOutput.tool_warning") as mock_warning: + with patch("aider.io.InputOutput.tool_warning") as mock_warning, \ + patch("aider.models.Model.set_reasoning_effort") as mock_set_reasoning: main( ["--model", "gpt-3.5-turbo", "--reasoning-effort", "3", "--yes", "--exit"], input=DummyInput(), @@ -743,6 +753,8 @@ class TestMain(TestCase): if "reasoning_effort" in call[0][0]: warning_shown = True self.assertTrue(warning_shown) + # Method should still be called by default + mock_set_reasoning.assert_called_once_with("3") @patch("aider.models.ModelInfoManager.set_verify_ssl") def test_no_verify_ssl_sets_model_info_manager(self, mock_set_verify_ssl): @@ -1014,3 +1026,65 @@ class TestMain(TestCase): self.assertEqual( coder.main_model.extra_params.get("thinking", {}).get("budget_tokens"), 1000 ) + + def test_check_model_accepts_settings_flag(self): + # Test that --check-model-accepts-settings affects whether settings are applied + with GitTemporaryDirectory(): + # When flag is on, setting shouldn't be applied to non-supporting model + with patch("aider.models.Model.set_thinking_tokens") as mock_set_thinking: + main( + [ + "--model", "gpt-4o", + "--thinking-tokens", "1000", + "--check-model-accepts-settings", + "--yes", "--exit" + ], + input=DummyInput(), + output=DummyOutput(), + ) + # Method should not be called because model doesn't support it and flag is on + mock_set_thinking.assert_not_called() + + # When flag is off, setting should be applied regardless of support + with patch("aider.models.Model.set_reasoning_effort") as mock_set_reasoning: + main( + [ + "--model", "gpt-3.5-turbo", + "--reasoning-effort", "3", + "--no-check-model-accepts-settings", + "--yes", "--exit" + ], + input=DummyInput(), + output=DummyOutput(), + ) + # Method should be called because flag is off + mock_set_reasoning.assert_called_once_with("3") + def test_model_accepts_settings_attribute(self): + with GitTemporaryDirectory(): + # Test with a model where we override the accepts_settings attribute + with patch("aider.models.Model") as MockModel: + # Setup mock model instance to simulate accepts_settings attribute + mock_instance = MockModel.return_value + mock_instance.name = "test-model" + mock_instance.accepts_settings = ["reasoning_effort"] + mock_instance.validate_environment.return_value = {"missing_keys": [], "keys_in_environment": []} + mock_instance.info = {} + mock_instance.weak_model_name = None + mock_instance.get_weak_model.return_value = None + + # Run with both settings, but model only accepts reasoning_effort + main( + [ + "--model", "test-model", + "--reasoning-effort", "3", + "--thinking-tokens", "1000", + "--check-model-accepts-settings", + "--yes", "--exit" + ], + input=DummyInput(), + output=DummyOutput(), + ) + + # Only set_reasoning_effort should be called, not set_thinking_tokens + mock_instance.set_reasoning_effort.assert_called_once_with("3") + mock_instance.set_thinking_tokens.assert_not_called()