chore(tests): stop leaking channels on cleanup failure#232
Closed
nijeesh-stream wants to merge 1 commit into
Closed
chore(tests): stop leaking channels on cleanup failure#232nijeesh-stream wants to merge 1 commit into
nijeesh-stream wants to merge 1 commit into
Conversation
The CI test suite creates ~94 channels per run (~50 sync + ~44 async)
across fixtures and explicit cases, but cleanup historically ran
through three escape hatches that silently leaked channels into the
shared test app:
1. channel fixture used client.delete_channels([cid], hard_delete=True),
which returns a task_id immediately. When the task queue is backed
up (the same backend slowness that breaks test_delete_channels) the
fixture says 'done' before the delete actually fires.
2. Every cleanup path swallowed exceptions: except Exception: pass.
Failures invisible.
3. test_query_drafts (sync + async) created a second channel,
channel2, and did not delete it on the failure path.
Stale channels drift across runs and eventually break unrelated tests
that query the shared app expecting clean state.
Three fixes:
* Swap client.delete_channels for the synchronous channel.delete(hard=True)
HTTP DELETE in every fixture. No task_id race; the channel is gone
before the fixture returns.
* Log cleanup exceptions to stderr ('[cleanup] channel <cid> failed:
...'). Visible in CI logs, doesn't fail the test.
* Add a session-scoped, autouse=True sweep that queries channels tagged
{'test': True, 'language': 'python'} at start and end of the run and
hard-deletes whatever is still hanging around. Self-healing across
runs; tag test_query_drafts's channel2 so the sweep catches it too.
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.
Summary
Audit: the CI suite creates ~94 channels per run (~50 sync + ~44 async) across fixtures and explicit test creations. Cleanup historically leaked them three ways:
channelfixture (scope="function", used by 37 sync + 31 async tests)fellowship_of_the_ringfixturetest_channel.pyexplicit creates (lines 32, 43, 322)test_draft.py:88/ asynctest_draft.py:88(channel2, no cleanup on failure)test_channel_batch_updater.py(4 tests × ch1+ch2, both suites)Three concrete leak vectors:
channelfixture usedclient.delete_channels([cid], hard_delete=True)— that returns atask_idand the fixture returns immediately. When the task queue is backed up (the same backend slowness that breakstest_delete_channels) the channel is still alive when the next test asserts on a clean app.except Exception: pass— failures invisible. CI logs show nothing when delete fails.test_query_drafts(sync + async) createdchannel2but only cleaned it up on the happy path — any assertion failure mid-test leaked it forever.Stale channels drift across runs and eventually break unrelated tests with stale-state assertions (e.g.
len(...) == 3 got 15).Fix
client.delete_channels(...)→channel.delete(hard=True)in every fixture. Synchronous HTTP DELETE, no task_id race.[cleanup] channel <cid> failed: <kind>: <msg>)._purge_stale_test_channelssession-scoped,autouse=Truefixture queries channels tagged{"test": True, "language": "python"}at session start and end and hard-deletes them. Tagtest_query_drafts'schannel2so the sweep catches it too.Test plan
uv run black— clean[cleanup]lines only appear when something legitimately failed; on a clean run there should be at most one[cleanup] swept N leaked test channelsline at session start (covering whatever the prior run left behind) and possibly one at session end (own-run strays, if any).