Skip to content

refactor(test): exercise real shutdown loop in lifespan tests#668

Open
mvillmow wants to merge 6 commits into
mainfrom
460-auto-impl
Open

refactor(test): exercise real shutdown loop in lifespan tests#668
mvillmow wants to merge 6 commits into
mainfrom
460-auto-impl

Conversation

@mvillmow

@mvillmow mvillmow commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Refactor TestLifespanShutdown.test_shutdown_waits_for_inflight_requests and test_shutdown_proceeds_after_timeout to exercise the real shutdown polling loop via async with lifespan(app) instead of duplicating it
  • Extend _make_mock_publisher() to set connect as AsyncMock so the lifespan entry path doesn't TypeError
  • Add proper cleanup (cache clear, state reset) in try/finally blocks to prevent state leakage across tests

Test plan

  • ✅ Both refactored tests pass and exercise the real production code path
  • ✅ Regression mutation (commenting out await publisher.disconnect()) causes both tests to fail
  • ✅ All 20 tests in test_shutdown.py pass (no regressions in TestShutdownMiddleware or TestInflightCounter which share _make_mock_publisher)
  • ✅ Lint and format clean

Closes #460

@mvillmow mvillmow left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests now drive the real lifespan drain loop via async with lifespan(app); correct & complete. Two minor nits: deprecated get_event_loop() and a flake-prone wall-clock upper bound.

Comment thread tests/test_shutdown.py
Comment thread tests/test_shutdown.py
@mvillmow mvillmow force-pushed the 460-auto-impl branch 2 times, most recently from 959d72f to f7449cb Compare June 28, 2026 16:11
@mvillmow mvillmow enabled auto-merge (squash) June 28, 2026 18:51
mvillmow and others added 6 commits June 28, 2026 18:09
…hutdown

Both test_shutdown_waits_for_inflight_requests and test_shutdown_proceeds_after_timeout
now drive the actual lifespan context manager via 'async with lifespan(app)' instead
of duplicating the polling loop from server.py. This ensures regression mutations to the
shutdown logic (e.g., wrong condition, missing disconnect) are caught by the tests.

Changes:
- Add 'patch' to top-level mock imports
- Extend _make_mock_publisher() to set 'connect' as AsyncMock so lifespan's
  _connect_with_retries call chain doesn't TypeError
- Refactor both tests to use 'async with lifespan(app)' with patched Publisher
- Clear get_settings cache before and after tests to ensure SHUTDOWN_TIMEOUT env is observed
- Use try/finally to reset _inflight and _shutdown_event regardless of assertion outcome

Tests now verify that disconnect is called at the exact moment _inflight reaches 0
(waits test) and that timeout deadlines are honored (timeout test).

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Signed-off-by: Claude Haiku 4.5 <4211002+mvillmow@users.noreply.github.com>
Signed-off-by: mvillmow <4211002+mvillmow@users.noreply.github.com>
Reviewed implementation for follow-ups per strict scope rules. No defects,
security issues, safety hazards, or critical bugs were introduced by the
refactoring. Two items (Python version compatibility and deprecation warnings)
were pre-existing and out of scope.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Signed-off-by: Claude Haiku 4.5 <4211002+mvillmow@users.noreply.github.com>
Signed-off-by: mvillmow <4211002+mvillmow@users.noreply.github.com>
Signed-off-by: Claude Haiku 4.5 <4211002+mvillmow@users.noreply.github.com>
Signed-off-by: mvillmow <4211002+mvillmow@users.noreply.github.com>
…ertion

Replace deprecated asyncio.get_event_loop() with asyncio.get_running_loop()
in test_shutdown_proceeds_after_timeout — required for Python 3.10+ coroutines.
Drop the flaky wall-clock upper bound (elapsed < timeout + 1.5); the lower
bound (elapsed >= timeout) is the real regression guard.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Claude Haiku 4.5 <4211002+mvillmow@users.noreply.github.com>
Signed-off-by: mvillmow <4211002+mvillmow@users.noreply.github.com>
Return properly formatted JSON follow-up response per template. No follow-ups
file as all pre-existing issues were addressed in the implementation plan review.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Signed-off-by: Claude Haiku 4.5 <4211002+mvillmow@users.noreply.github.com>
Signed-off-by: mvillmow <4211002+mvillmow@users.noreply.github.com>
This internal follow-up note was committed by the automation loop but is
unrelated to issue #460 and tripped the end-of-file-fixer pre-commit hook
(missing trailing newline). Remove it and gitignore the .claude-followup-*.md
pattern so it cannot recur. Addresses lint failure.

Signed-off-by: Claude Haiku 4.5 <4211002+mvillmow@users.noreply.github.com>
Signed-off-by: mvillmow <4211002+mvillmow@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestLifespanShutdown tests duplicate the shutdown polling loop

1 participant