test: re-enable recoverable skipped tests (+ fix latent LazySpan bug)#719
Closed
wbarnha wants to merge 1 commit into
Closed
test: re-enable recoverable skipped tests (+ fix latent LazySpan bug)#719wbarnha wants to merge 1 commit into
wbarnha wants to merge 1 commit into
Conversation
Sweep the suite's legacy `@pytest.mark.skip("Needs fixing")` tests and
re-enable the ones that can be made green, fixing a real bug found along the
way.
* fix(aiokafka): bind the lazy span's replacement `finish`. `LazySpan.finish`
was assigned to `span.finish` as a raw (unbound) function, so the later
`span.finish()` call raised "missing 1 required positional argument:
'self'". Lazy spans are used whenever tracing is enabled, so this was a
latent crash; the skipped `test_transform_span_*` tests were hiding it.
Re-enables the four transform-span tests.
* test(aiokafka): the two stream-idle VEP tests called the 2-arg
`_make_slow_processing_error`; it now takes `setting`/`current_value` and
bakes the explanation into the message, so `log.error` no longer receives
those as kwargs. Updated the expectations and re-enabled them.
* test(agent): `test_execute_actor__cancelled_running` (was "Fix is TBD")
passes as-is now -- skip rot -- re-enabled.
The remaining previously-skipped tests need per-test work beyond a re-enable
and stay skipped, but with precise reasons replacing "Needs fixing": the
fetch-timeout VEP asserts (driver behaviour drift) and the `_commit` mock
tests in test_aiokafka.py. Tests needing live infrastructure or a
dependency API refresh (redis cache backend, rocksdb `Options`, the
transaction-manager mocks, the CLI compat-option test, tests/consistency's
stale aiokafka import) are left as they were.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
Member
Author
|
Closing in favour of #723, which now contains this work rebased directly onto Generated by Claude Code |
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
1 participant
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
Follow-up to #716. Sweeps the suite's 20 legacy unconditional
@pytest.mark.skip("Needs fixing"/"Fix is TBD")tests, re-enabling every one that can be made green and fixing a real bug a skip was hiding. Stacked onclaude/pypy-agen-cleanup(#716) — retarget tomasteronce that merges.Re-enabled (7 tests)
faust/transport/drivers/aiokafka.py:_transform_span_lazyassigned the raw, unboundLazySpan.finishtospan.finish, so the laterspan.finish()raisedTypeError: finish() missing 1 required positional argument: 'self'. Lazy spans are used whenever tracing is enabled, so this was a latent crash — the four skippedtest_transform_span_*tests were masking it. Bound the method to the span; 4 tests re-enabled.test_aiokafka.pystream-idle VEP (2):_make_slow_processing_errorgrewsetting/current_valueparams and now bakes the explanation into the message, solog.errorno longer gets those as kwargs. Updated the two stale expectations.test_agent.py::test_execute_actor__cancelled_running(1): "Fix is TBD" — passes as-is now (skip rot).Attempted, still skipped — but with precise reasons (was "Needs fixing")
test_aiokafka.pyfetch-timeout VEP asserts ×3 —verify_event_pathno longer emits the error() under those conditions (driver behaviour drift);_commitmock tests ×3 — offsets no longer flow to the wrappedAIOKafkaConsumer.commitin that harness. Both need real test re-derivation.Attempted, left as-is (need infra / dependency-API work)
Documented here rather than re-skipped in code:
test_cache.pyredis scheme (2)StrictRedistest_rocksdb.py::test_open_rocksdictrocksdict'sOptions()no longer takes constructor argstest_consumer.pyTransactionManager (2)stop_transactioncall-count / expected-calls drifttest_base.py::test_compat_optiontest_consistency.pyGroupCoordinatorRequest_v0removed); this dir isn't in the CI suiteVerification
tests/unit/transport+tests/unit/agents/test_agent.py— 598 passed, 16 skipped, 0 failed on CPython; flake8/black/isort clean.🤖 Generated with Claude Code
Generated by Claude Code