Skip to content

test: re-enable recoverable skipped tests (+ fix latent LazySpan bug)#728

Merged
wbarnha merged 4 commits into
masterfrom
claude/reenable-remaining-skipped-tests
Jul 21, 2026
Merged

test: re-enable recoverable skipped tests (+ fix latent LazySpan bug)#728
wbarnha merged 4 commits into
masterfrom
claude/reenable-remaining-skipped-tests

Conversation

@wbarnha

@wbarnha wbarnha commented Jul 21, 2026

Copy link
Copy Markdown
Member

Description

Re-opens #723 onto master. #723 was merged into its stale stacked-parent branch (claude/pypy-agen-cleanup) instead of master, so its content never reached master. This branch is the same two commits rebased directly onto master, independent of any chain.

Re-enables the recoverable @pytest.mark.skip tests across the aiokafka driver, agents, and CLI, and fixes a real bug a skip was hiding.

Re-enabled (test-only unless noted)

faust/transport/drivers/aiokafka.py + test_aiokafka.py

  • Real bug fix: _transform_span_lazy assigned the unbound LazySpan.finish to span.finish, so span.finish() crashed with missing 1 required positional argument: 'self'. Lazy spans run whenever tracing is enabled — a latent crash the 4 test_transform_span_* tests were masking. Bound it to the span. (source fix)
  • Test_VEP_stream_idle_* (2) — updated for the _make_slow_processing_error(msg, causes, setting, current_value) signature.
  • Test_VEP_no_fetch_since_start::test_timed_out — set state_value().timestamp = None so the NO_FETCH_SINCE_START branch can fire.
  • test__commit / __CommitFailedError / __IllegalStateError (3) — point the mocked assignment() at TP1; drop the stale supervisor.wakeup() asserts (removed from _commit in 2020).

test_agent.pytest_execute_actor__cancelled_running passes as-is now (skip rot).

test_base.pytest_compat_option now grabs the real callback closure and checks its behaviour against faust/cli/base.py.

Left skipped — genuine source gap (flagged, not forced)

Test_VEP_no_response_since_start::test_timed_out and Test_VEP_no_recent_response::test_timed_out stay skipped: SLOW_PROCESSING_NO_RESPONSE_SINCE_START / NO_RECENT_RESPONSE are defined in the driver but never emitted. Out of scope for a test-only change; worth a follow-up.

Verification

test_aiokafka.py + test_agent.py + test_base.py295 passed, 3 skipped, 0 failed on CPython; flake8/black/isort clean.

🤖 Generated with Claude Code


Generated by Claude Code

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.02%. Comparing base (69c15a3) to head (665dfb5).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #728      +/-   ##
==========================================
+ Coverage   94.37%   95.02%   +0.65%     
==========================================
  Files         104      104              
  Lines       11158    11158              
  Branches     1203     1203              
==========================================
+ Hits        10530    10603      +73     
+ Misses        529      461      -68     
+ Partials       99       94       -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wbarnha
wbarnha enabled auto-merge July 21, 2026 11:12
wbarnha and others added 2 commits July 21, 2026 11:18
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
Continues the skipped-test sweep from the parent branch.

aiokafka (tests/unit/transport/drivers/test_aiokafka.py):
* Test_VEP_no_fetch_since_start::test_timed_out -- the base _consumer fixture
  stamps a current poll timestamp, so the NO_FETCH_SINCE_START branch (fires
  only when the partition has no poll timestamp yet) could never trigger. Set
  state_value().timestamp = None (the real "no fetch request ever sent"
  condition) and tighten the asserted tp. Now green.
* test__commit / __CommitFailedError / __IllegalStateError -- _commit filters
  offsets by self.assignment() before building the payload; the fixture's
  assignment didn't contain TP1, so every offset was filtered out
  (commit({})). Point the mocked assignment at TP1 so offsets flow through.
  Also dropped the stale supervisor.wakeup() assertions -- those calls were
  intentionally removed from _commit's error handlers in 2020 (commit
  8d6758f); the crash(exc)/returns-False assertions are kept.

CLI (tests/unit/cli/test_base.py):
* test_compat_option -- the old test asserted on a Mock (option(ctx)._callback
  is a Mock attribute, never 33). compat_option wires the real callback via the
  click `callback=` kwarg; grab it from opt.kwargs["callback"] and assert its
  actual behaviour against faust/cli/base.py: ensure_object(State), value
  returned unchanged, stored on the State only when the previous value is None
  and the value differs from the option default.

Two response-staleness VEP tests stay skipped, now with an accurate reason:
SLOW_PROCESSING_NO_RESPONSE_SINCE_START and SLOW_PROCESSING_NO_RECENT_RESPONSE
are defined in the driver but never emitted (the broker-response-staleness
tracking was dropped), so they cannot pass without re-adding that source
feature -- out of scope for a test-only change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHPL4VFWQRQPpjR1gXSKyL
@wbarnha
wbarnha force-pushed the claude/reenable-remaining-skipped-tests branch from 055d087 to db3c72d Compare July 21, 2026 11:18
@wbarnha
wbarnha added this pull request to the merge queue Jul 21, 2026
Merged via the queue into master with commit d875380 Jul 21, 2026
25 checks passed
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