Skip to content

[WIP] Fix incorrect test assertion in test_user_exporters_plus_env_exporter#94

Merged
CorentinGS merged 4 commits into
masterfrom
copilot/fix-user-exporters-test-assertion
Jan 20, 2026
Merged

[WIP] Fix incorrect test assertion in test_user_exporters_plus_env_exporter#94
CorentinGS merged 4 commits into
masterfrom
copilot/fix-user-exporters-test-assertion

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 20, 2026

  • Understand the problem: The test expects 6 processors (1 user exporter + 1 env exporter + 4 Basalt processors) but only 5 are added (1 user exporter + 4 Basalt processors) because the env exporter is only added when no user exporters are provided
  • Update line 135 in tests/observability/test_multi_exporters.py: Change assertion from 6 to 5
  • Update line 133 comment to clarify expected count: "1 exporter + 4 Basalt processors = 5 total processors"
  • Add note explaining environment exporter is only used as fallback
  • Run the test to verify the fix - PASSED
  • Run all tests in the test file to ensure no regressions - ALL 13 TESTS PASSED
  • Address code review feedback: Update test docstring to reflect actual behavior (user exporters replace environment exporter, not combined with it)
  • Address code review feedback: Update line 130 comment from "Verify both exporters were used" to "Verify user exporter was used"
Original prompt

Fix incorrect test assertion in test_user_exporters_plus_env_exporter

Problem

The test test_user_exporters_plus_env_exporter in tests/observability/test_multi_exporters.py is failing with:

AssertionError: 5 != 6

The test expects 6 processors but only 5 are actually added.

Root Cause

The test incorrectly assumes that when a user provides an exporter, the environment exporter is also added (resulting in 2 exporters + 4 Basalt processors = 6 total).

However, examining the code in basalt/observability/instrumentation.py (lines 228-229), the environment exporter is only added if no user exporters were provided:

# Add environment exporter ONLY if no user exporters were provided
if user_exporters is None and env_exporter:
    exporters_list.append(env_exporter)

Since the test provides a user exporter (InMemorySpanExporter()), the condition user_exporters is None is False, so the environment exporter is NOT added.

Actual Processor Count

  1. 1 user exporter processor (from InMemorySpanExporter())
  2. 4 Basalt processors (Context, CallEvaluator, ShouldEvaluate, AutoInstrumentation)

Total: 5 processors

Solution

Update the test assertion and comments to reflect the correct behavior:

File: tests/observability/test_multi_exporters.py

Lines 130-135:

# Verify both exporters were used
provider = manager._tracer_provider
self.assertIsInstance(provider, TracerProvider)
# Should have 1 exporter + 4 Basalt processors = 5 total processors
# Basalt processors: Context, CallEvaluator, ShouldEvaluate, AutoInstrumentation
# Note: Environment exporter is only used if no user exporter is provided
self.assertEqual(len(provider._active_span_processor._span_processors), 5)

Changes Required

  1. Change the assertion from self.assertEqual(..., 6) to self.assertEqual(..., 5) on line 135
  2. Update the comment on line 133 to clarify that we expect "1 exporter + 4 Basalt processors = 5 total processors"
  3. Add a note explaining that the environment exporter is only used as a fallback when no user exporter is provided

Additional Note

The test name test_user_exporters_plus_env_exporter is somewhat misleading as it suggests both exporters are used together, but the actual behavior is that the environment exporter serves as a fallback only.

This pull request was created from Copilot chat.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits January 20, 2026 22:08
Co-authored-by: CorentinGS <43623834+CorentinGS@users.noreply.github.com>
Co-authored-by: CorentinGS <43623834+CorentinGS@users.noreply.github.com>
Co-authored-by: CorentinGS <43623834+CorentinGS@users.noreply.github.com>
@CorentinGS CorentinGS marked this pull request as ready for review January 20, 2026 22:11
Copilot AI requested a review from CorentinGS January 20, 2026 22:12
Copilot stopped work on behalf of CorentinGS due to an error January 20, 2026 22:12
@CorentinGS CorentinGS merged commit 8891b90 into master Jan 20, 2026
5 checks passed
@CorentinGS CorentinGS deleted the copilot/fix-user-exporters-test-assertion branch January 20, 2026 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants