test(consumer): fix TransactionManager stop/start-transaction assertions#722
Merged
Conversation
test__stop_transactions and test_start_transactions were skipped "Needs
fixing": they used `assert_called_once_with([call("0-0"), call("1-0")])`, which
asserts a single producer call taking a list of Call objects -- not what the
code does. _stop_transactions/_start_transactions drive the producer once per
transaction id (stop_transaction / maybe_begin_transaction with a single string
arg each), so for two ids there are two calls.
Assert that faithfully: `assert_has_calls([call("0-0"), call("1-0")],
any_order=True)` plus `call_count == 2`. Ordering isn't asserted because the
ids come from a set and traced_from_parent_span interleaves shortlabel mock
accesses between the real calls. Only the Kafka producer is mocked.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #722 +/- ##
==========================================
+ Coverage 94.18% 94.25% +0.07%
==========================================
Files 104 104
Lines 11154 11154
Branches 1202 1202
==========================================
+ Hits 10505 10513 +8
+ Misses 548 540 -8
Partials 101 101 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
wbarnha
enabled auto-merge
July 21, 2026 11:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Re-enables
TestTransactionManager::test__stop_transactionsandtest_start_transactions, skipped "Needs fixing".Both used
assert_called_once_with([call("0-0"), call("1-0")])— which asserts a single producer call whose only argument is a list ofCallobjects. That's not what the code does:_stop_transactions/_start_transactionsdrive the producer once per transaction id (stop_transaction/maybe_begin_transaction, each with a single string arg), so for two ids there are two calls (the original even had commented-out notes about "extra garbage" calls it couldn't reconcile).Fix (test-only): assert it faithfully —
assert_has_calls([call("0-0"), call("1-0")], any_order=True)pluscall_count == 2. Ordering isn't asserted because the ids come from a set andtraced_from_parent_spaninterleavesshortlabelmock accesses between the two real calls. Only the Kafka producer is mocked; the manager logic runs for real.Verification
tests/unit/transport/test_consumer.py→ 115 passed; flake8/black/isort clean. Produced under adversarial faithfulness review (confirmed the old assertion was genuinely broken and the new one reflects real behaviour).🤖 Generated with Claude Code
Generated by Claude Code