test: the last process-wide pool clears go — the restart seed test clears its hosts' own pool and proves it, the composition tests never open a database to clear, and the guard allows no file to clear every pool #54
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "test/pool-clears-own-db"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
PR #42 changed every Integration teardown to clear only its own database's pool. A process-wide
SqliteConnection.ClearAllPools()races the test classes that xunit runs in parallel, and it had caused a reproduced flake:ObjectDisposedExceptiononSQLitePCL.sqlite3insideOpen. PR #42 left two callers, listed as pending in the guard. This PR removes both and empties the list. Everything touched is undertests/Tsumugi.Integration.Tests/, and no src, ADR, docs, seed or migration changes.SeedDevDataTests.A_restart_with_the_flag_on_seeds_nothing_more_and_says_so:TsumugiDbContextconnection string while the host is alive;SQLITE_CANTOPEN(14), which shows nothing of that database stayed pooled.ConnectionStringOf/ClearPoolOf/AssertNothingPooled) follow the ones inVolumeStartupValidatorTests.ProviderCompositionTestsresolve services and never open the database.AddInfrastructureonly registersUseSqlite, nothing migrates or queries, and SQLite does not create the parent directory. So the test root is never made and no connection was ever pooled, and a per-database clear there would pass whether or not it ran.CANTOPENif it never made the directory.SqliteBackedTestTeardownTests.No_integration_test_clears_every_poolreplaces the Pending ratchet. An empty result proves nothing by itself, so the test also checks that the guard can actually fire:nameof;ClearsEveryPool()) runs through the same scan function that reads each file's text. It must find exactly["SqliteBackedTest.cs"], so a scan that matched paths, read the same file every time, or read nothing would fail.base.Dispose()comment now says the base clears only this database's pool. The comment on the WAL reader says the reader keeps the WAL whatever happens to EF's pooled connection; the writer and the tests open everything else withPooling=false.No ADR covers test teardown, so none is amended.
Verification
tsumugi-*directories were left in/tmp. BlazorAdapter, Domain, UseCases and Kagaku.Flow were not run because this PR changes no file they build.ClearPoolOf(connectionString)→ the restart test (Assert.Throws() Failure: No exception was thrown).dataPath + "/."(same file, different pool key) → the restart test (Strings differ). A control run with theAssert.Equalremoved passed, so the Equal is what catches a second pool leaking.EnsureCreated→ that test's teardown.ClearAllPools()added back to the composition teardown → the guard ([Provider/ProviderCompositionTests.cs]).ClearAllPools ()with a space in SeedDevDataTests → the guard ([SeedDevDataTests.cs]).ClearAllPoolz→Assert.Matches.SearchOption.TopDirectoryOnly→ the subfolder control. Thebin/filter inverted → the scan controls.SqliteConnection.ClearAllPools();planted in SeedDevDataTests'finally.call.IsMatch(path)) →Assert.Equal() Failure: Collections differ.SqliteBackedTest.csfor every path →Collections differ.string.Empty→Collections differ.ClearsEveryPoolz→Collections differ.Files clearing every SQLite pool: [SeedDevDataTests.cs]. A plant inProvider/→[Provider/ProviderCompositionTests.cs].TopDirectoryOnlywith theProvider/plant → the subfolder control. Thebin/filter inverted → the subfolder control. The forbidden regex changed toClearAllPoolz→Assert.Matches.CANTOPENif it did not create the root. What the review found, and what was done:ClearAllPools()planted. The fix routes the forbidden pattern and a control pattern through one scan function. The control must find exactlySqliteBackedTest.cs, which kills all three variants (listed above)./tmp. The reviewer marked this optional. Cleaning up in that branch would add code that runs only when a test is already failing, so no green test could catch its removal. The residue is one directory, left only by a run that is already red, and it should be removed by hand in that case.GatedAppFactory.Disposedeletes its data directory without clearing its host's pool. That leaks idle handles to unlinked files for the rest of the run. It is not a flake, since it clears nothing process-wide.CANTOPENcheck now exists in four files and could become one shared helper.SqlitePoolTeardown'sDisableParallelizationcould be dropped now that the guard forbids global clears.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagaku.Flow - 99.7%
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__ColorToken_2
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__Socket_1
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__TypeId_0
Kagaku.Flow.OpenRouter - 100%
Kagaku.Flow.Testing - 100%
Tsumugi.BlazorAdapter - 96%
Tsumugi.Domain - 96.3%
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__DigitsAndSeparators_2
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__LineBreak_0
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__NumberToken_1
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__Plain_4
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__ThousandsGroup_3
Tsumugi.Infrastructure - 97.9%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 98.5%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ The last two process-wide pool clears, hunted down one by one until the pending list is empty? PR #42 set the ratchet and this PR finishes the cut — and you didn't just delete the calls, you made each deletion prove itself. The guard that once trusted a shrinking list now refuses to believe its own empty result until a control pattern walks the very same scan path. That's the kind of paranoid correctness that makes my wings flutter~ ♡
Verdict: ✅ Looks good to me~
No blocking issues. And I didn't take that on faith — I brought my own knives.
🔬 What I verified myself
b90a8bb— PR minutes old, no bot comment yet): Release build 0w/0e; Integration 513/513, digit-exact against your claim, run green twice after my probes; zerotsumugi-*residue in/tmpafter green runs.git diff --quietpristine at head):SqliteConnection.ClearAllPools();planted inSeedDevDataTests' finally → the guard failed withFiles clearing every SQLite pool: [SeedDevDataTests.cs]. Exactly the plant, exactly the file. ♪ClearPoolOfneutered (clear commented out) → the restart test failedAssert.Throws() Failure: No exception was thrown— digit-identical to your own probe report, dear author. The reopen-refused check is load-bearing.string.Emptyinstead of each file →Expected: ["SqliteBackedTest.cs"] Actual: []. The round-0 fix holds: an empty scan cannot slip through anymore.a composition test opened its database and left it behind. The tripwire is real, and xunit (2.9.3) surfaces Dispose asserts with a proper stack frame — I checked the mechanics, not just the red.dataPath + "/."→Assert.Equalfailed at the connection-string seam (Strings differ). The pool-key equality check catches a second pool leaking, precisely as designed.ConnectionStringOf/ClearPoolOf/AssertNothingPooledare verbatim theVolumeStartupValidatorTestshelpers — same null-guard, samefinallyclear, same comment shape. Nothing inventive, nothing drifting. ♡AddInfrastructureonly doesAddDbContext+UseSqlite— lazy registration, no open, no directory creation — andProviderCompositionTestsresolves services without ever touching the database. The teardown'sAssert.False(Directory.Exists(_root))is therefore a sound tripwire, not a vacuous assert.BackupWriterTests): the writer's snapshot and read connections arePooling = falseinSqliteSnapshot.cs— the corrected comments now tell the truth. Misleading comments get my knife; these needed no cut~ClearsEveryPoolexists in exactly one file (SqliteBackedTest.cs), soAssert.Equal(["SqliteBackedTest.cs"], …)can't rot silently.💡 Little ideas (non-blocking)~
Tsumugi.Integration.Tests.csprojlinks..\Shared\MarkupCorpus.csinto the compile from outside the project directory — invisible toEnumerateFilesunder the project root. Today that file is a markup-corpus reader and would never plausibly grow pool code, and the scan is defense-in-depth beside the behavioral teardown tests, so this is a nicety, not a hole I'd block on. If you ever want the ratchet airtight: also scan../Shared/*.cs, or assert the csproj's linkedCompileitems are all under the project root./tmpresidue from a failing composition teardown — I produced one concretely with my probe (and cleaned it up~). Agreeing with your independent reviewer: code that only runs when a test is already red buys nothing. Leave it.GatedAppFactory.Dispose's directory delete, the reopen-refused check now living in four files, droppingDisableParallelization) — all correctly out of this PR's scope. For the record, I confirmed theGatedAppFactoryleftover is real (delete without a clear, line 142-144).✅ What I liked~
Assert.Contains(sources, …)pins that the scan reaches subfolders; routing both the forbidden pattern and a control through oneCalling(Regex)closure kills the match-the-path / read-one-fixed-file / read-nothing evasion class structurally — the control must find exactly one file, so the scan's every moving part is pinned from inside the same test that enforces the rule. AndAssert.Matches(ClearsEveryPool(), $"{nameof(SqliteConnection)}.{nameof(SqliteConnection.ClearAllPools)}();")building the sample fromnameofso a rename can't orphan it? Oh, that's deliciously paranoid~ ♡Assert.Equal(connectionString, ConnectionStringOf(second))) — quietly the sharpest line in the PR. It's what makes "one clear covers both hosts" a proven invariant instead of a hope.base.Dispose()comment now stating exactly what the base does. This is what a change description should look like.PR #42's list is empty, and this time the emptiness means something. Merge it when you like~ fufu ♪
Automated review by Jibril · 2026-09-26
CI/CD: absent for head SHA
b90a8bb(no coverage bot comment yet) · Local checks: build 0w/0e; Integration 513/513 ×2 green post-probes; 5/5 mutation probes killed; 1 stale-binary false-green caught + redone;/tmpclean