Skip to content

Test that a server which never answers costs one report, not the sender - #29

Merged
dmccoystephenson merged 1 commit into
mainfrom
trace/test-per-request-timeout
Oct 31, 2026
Merged

dmccoystephenson merged 1 commit into
mainfrom
trace/test-per-request-timeout

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

  • A Stage B (unit-test expansion) cycle: a characterization test is added for the per-request bound in _send, which no existing test asserted.
  • test_a_server_that_never_answers_costs_one_report_not_the_sender shortens TraceClient.TIMEOUT_SECONDS to 0.5 s, holds the loopback server's first request unanswered, and checks three things: the hung request is logged as could not deliver, the next report is still delivered, and every log record is at DEBUG.
  • Before this, a regression that dropped or raised the urlopen timeout would have passed the suite. The existing hang test only bounds close(), not the sender.
  • Only tests change. No production code or docs are touched.

Test plan

  • python3 -m unittest -v locally (Python 3.8.10): Ran 68 tests, OK
  • Mutation check: with urlopen(..., timeout=30) in place of self.TIMEOUT_SECONDS, the new test failed (FAILED (failures=1)); with the source restored, it passed
  • CI test (3.8), test (3.10), test (3.12) green

Deferred

Java parity

No Java-visible behavior changed. This adds a test only.

No tracking issue: the gap was found during triage.

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

Each request is bounded by TIMEOUT_SECONDS; nothing asserted it. A server
that accepts the connection and hangs must time out that report, log it at
DEBUG, and leave the sender free to deliver the next one.

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

Copy link
Copy Markdown
Member Author

Self-review rubric (one pass, anchored on CI run 41180910676):

  • Scope: PASS. gh pr diff 29 --name-only lists only tests/test_trace_client.py (+20 lines, one new test).
  • Tests-new: PASS (not applicable). No new public names were added.
  • Tests-fix: PASS (not applicable as a bug fix). This is a characterization test. A mutation check was still run: with the urlopen timeout replaced by 30, the new test failed (FAILED (failures=1)), and with the source restored it passed.
  • Sibling structure: PASS. The test sits in TraceClientTest, uses _Capture.release to simulate a hanging server, uses mock.patch.object and self.log, closes its client, and has explanatory assertion messages.
  • Sibling renames: PASS (not applicable). Nothing was renamed.
  • Docs: PASS. No behavior changed. The README "Bounded" and "never raises" rows already describe what the test asserts.
  • Issue resolution: PASS (not applicable). No Closes. The body records the triage-found gap and the deferral of The version tag is added even when it pushes an event past MAX_TAGS #16.
  • CI: PASS. test (3.8), test (3.10) and test (3.12) are all green on the PR head.
  • Stdlib-only: PASS. No added import lines.
  • Py3.8 floor: PASS. test (3.8) is green, and the local run was also on 3.8.10.
  • Never-raises / never-blocks: PASS (not applicable). No production code changed.
  • Log level: PASS. No _LOG calls were added. The test itself asserts that every record is DEBUG.
  • Version agreement / Exports: PASS (not applicable). Neither was touched.
  • Java parity noted: PASS. The body states that no Java-visible behavior changed.

Judgment call for a reader: the 10 s polling deadline is twice the default timeout and twenty times the patched one, so it should be generous enough for a slow runner. Patching the class attribute does not change close()'s default timeout, because that default was bound when the function was defined. close() therefore still allows the full 5 s for "second" to drain.

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


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit e442268 into main Oct 31, 2026
3 checks passed
@dmccoystephenson
dmccoystephenson deleted the trace/test-per-request-timeout branch October 31, 2026 07:32
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