Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 18 additions & 19 deletions tests/test_litellm/integrations/test_langfuse.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,22 +256,21 @@ def test_log_langfuse_v2_handles_null_usage_values(self):
Test that _log_langfuse_v2 correctly handles None values in the usage object
by converting them to 0, preventing validation errors.
"""
# Create fresh mocks for this test to avoid state pollution from setUp's side_effect
# The setUp configures trace.side_effect which can interfere with return_value
mock_trace = MagicMock()
mock_generation = MagicMock()
mock_generation.trace_id = "test-trace-id"
# Reset the mock to ensure clean state
self.mock_langfuse_client.reset_mock()
self.mock_langfuse_trace.reset_mock()
self.mock_langfuse_generation.reset_mock()

# Re-setup the trace and generation chain with clean state
self.mock_langfuse_generation.trace_id = "test-trace-id"
mock_span = MagicMock()
mock_span.end = MagicMock()

mock_trace.generation.return_value = mock_generation
mock_trace.span.return_value = mock_span

mock_client = MagicMock()
mock_client.trace.return_value = mock_trace

# Use our fresh mock client
self.logger.Langfuse = mock_client
self.mock_langfuse_trace.span.return_value = mock_span
self.mock_langfuse_trace.generation.return_value = self.mock_langfuse_generation

# Ensure trace returns our mock
self.mock_langfuse_client.trace.return_value = self.mock_langfuse_trace
Comment on lines +260 to +272
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant return_value due to active side_effect

reset_mock() on a parent MagicMock resets call counts recursively but does not clear side_effect or return_value on child mocks. The setUp method sets self.mock_langfuse_client.trace.side_effect = _trace_side_effect (line 56), and this side_effect survives the reset_mock() call on line 260.

When both side_effect and return_value are set on a mock, Python's unittest.mock gives side_effect precedence. So the return_value set on line 272 is effectively ignored — the side_effect from setUp is what actually runs when .trace() is called.

This happens to work correctly because _trace_side_effect returns self.mock_langfuse_trace, which is the same object being set as return_value. However, for clarity and to avoid confusion for future maintainers, consider explicitly clearing the side_effect:

Suggested change
self.mock_langfuse_client.reset_mock()
self.mock_langfuse_trace.reset_mock()
self.mock_langfuse_generation.reset_mock()
# Re-setup the trace and generation chain with clean state
self.mock_langfuse_generation.trace_id = "test-trace-id"
mock_span = MagicMock()
mock_span.end = MagicMock()
mock_trace.generation.return_value = mock_generation
mock_trace.span.return_value = mock_span
mock_client = MagicMock()
mock_client.trace.return_value = mock_trace
# Use our fresh mock client
self.logger.Langfuse = mock_client
self.mock_langfuse_trace.span.return_value = mock_span
self.mock_langfuse_trace.generation.return_value = self.mock_langfuse_generation
# Ensure trace returns our mock
self.mock_langfuse_client.trace.return_value = self.mock_langfuse_trace
# Reset the mock to ensure clean state
self.mock_langfuse_client.reset_mock()
self.mock_langfuse_trace.reset_mock()
self.mock_langfuse_generation.reset_mock()
# Re-setup the trace and generation chain with clean state
self.mock_langfuse_generation.trace_id = "test-trace-id"
mock_span = MagicMock()
mock_span.end = MagicMock()
self.mock_langfuse_trace.span.return_value = mock_span
self.mock_langfuse_trace.generation.return_value = self.mock_langfuse_generation
# Clear side_effect from setUp so return_value takes effect
self.mock_langfuse_client.trace.side_effect = None
self.mock_langfuse_client.trace.return_value = self.mock_langfuse_trace
self.logger.Langfuse = self.mock_langfuse_client

self.logger.Langfuse = self.mock_langfuse_client

with patch(
"litellm.integrations.langfuse.langfuse._add_prompt_to_generation_params",
Expand Down Expand Up @@ -325,13 +324,13 @@ def mock_get(key, default=None):
)
except Exception as e:
self.fail(f"_log_langfuse_v2 raised an exception: {e}")

# Verify that trace was called first
mock_client.trace.assert_called()
self.mock_langfuse_client.trace.assert_called()

# Check the arguments passed to the mocked langfuse generation call
mock_trace.generation.assert_called_once()
call_args, call_kwargs = mock_trace.generation.call_args
self.mock_langfuse_trace.generation.assert_called_once()
call_args, call_kwargs = self.mock_langfuse_trace.generation.call_args

# Inspect the usage and usage_details dictionaries
usage_arg = call_kwargs.get("usage")
Expand Down
Loading