Skip to content

Test the opt-out mapping, delivery order and the queue-full log - #21

Merged
dmccoystephenson merged 1 commit into
mainfrom
trace/test-opt-out-mapping-order-and-drop-log
Oct 21, 2026
Merged

dmccoystephenson merged 1 commit into
mainfrom
trace/test-opt-out-mapping-order-and-drop-log

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

Stage B (unit-test expansion) cycle: three behaviours the suite did not yet check now have characterization tests. Production code is unchanged.

  • test_environment_opts_out_reads_only_the_mapping_it_is_given: with no argument or None, environment_opts_out reads os.environ. An explicit mapping (even {}) replaces the process environment rather than adding to it. Its values are trimmed and compared case-insensitively, the same as the environment's.
  • test_a_dropped_report_is_logged_at_debug_with_its_name: once the queue and the sender are both full, a dropped report is logged once as [trace] queue full, dropped <name>, at DEBUG only. The README's "Bounded" and "Never raises" rows depend on this.
  • test_reports_are_delivered_in_the_order_they_were_made: because there is one sender thread, the server sees events in the order they were reported.

No tracking issue: these gaps were found while comparing trace_client.py against tests/test_trace_client.py.

Skipped: #16 stays open. It needs an owner decision on the MAX_TAGS / version promise and a coordinated change in trace-client-java, as its existing comment records.

No Java-visible behaviour changed; trace-client-java needs no matching change.

Test plan

  • python3 -m unittest -v locally on Python 3.8.10: Ran 62 tests, OK
  • CI test (3.8), test (3.10), test (3.12) green

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code


drafted by Claude on behalf of Daniel Stephenson

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Self-review rubric:

  • Scope: PASS. The only file changed is tests/test_trace_client.py (+34 lines), and production code is untouched (git diff --stat origin/main).
  • Tests-new: PASS (not applicable). No public method was added; the three new tests cover existing public behaviour.
  • Tests-fix: PASS (not applicable). This is a characterization-only cycle with no bug fix to revert.
  • Sibling structure: PASS. The tests sit in TraceClientTest next to related tests and reuse _server/_Capture, self.log and mock.patch.dict. Each client built in the tests is close()d.
  • Sibling renames: PASS (not applicable). Nothing was renamed.
  • Docs: PASS. The tests assert what the README already says (one daemon thread; DEBUG-only logging on drops; the opt-out values are trimmed and case-insensitive). No doc needs a change.
  • Issue resolution: PASS (not applicable). There is no Closes. The version tag is added even when it pushes an event past MAX_TAGS #16's skip reason is in the PR body.
  • CI: PASS. test (3.8), test (3.10) and test (3.12) are all green on the PR head.
  • Stdlib-only: PASS. No new imports.
  • Py3.8 floor: PASS. test (3.8) is green, and the local run used Python 3.8.10 (Ran 62 tests, OK).
  • Never-raises / never-blocks, log level, version agreement, exports: PASS (not applicable). trace_client/ is unchanged.
  • Java parity noted: PASS. The PR body says no Java-visible behaviour changed.

Observations (judgment, not blocking):

  • tests/test_trace_client.py:528: test_reports_are_delivered_in_the_order_they_were_made relies on close()'s default 5 s drain delivering all 20 loopback requests. A badly stalled runner could deliver fewer, and the test would then fail on a list mismatch rather than hang. This is the same pattern as test_close_sends_what_was_just_queued_before_stopping, which sends 30, so the risk is considered acceptable.
  • tests/test_trace_client.py:513: test_a_dropped_report_is_logged_at_debug_with_its_name sends QUEUE_CAPACITY + 1 filler reports, so the named report is dropped however many reports the sender has already taken (at most one, because the server is held). This makes it deterministic.

Ready to merge: tests only, CI green, and no do-not-auto-merge path touched.

This self-review was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit 8918065 into main Oct 21, 2026
3 checks passed
@dmccoystephenson
dmccoystephenson deleted the trace/test-opt-out-mapping-order-and-drop-log branch October 21, 2026 09:59
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.

1 participant