Give each test run its own temp directory (BL-16664)

[JohnThomson]

Our C# tests name their scratch folders after themselves — about 180 calls of the form new TemporaryFolder("SomeFixtureTests") — and those names resolve to machine-global paths. That was harmless when one person ran the suite at a time. It is not harmless now that agents work in several worktrees at once, because TemporaryFolder's constructor deletes any existing folder of that name before creating it:

public TemporaryFolder(string name)
{
    _path = Path.Combine(Path.GetTempPath(), name);
    if (Directory.Exists(_path))
        DeleteFolderThatMayBeInUseAndIfNotFailSilently(_path);   // <-- someone else's live folder
    Directory.CreateDirectory(_path);
}

So one run's setup silently deletes another run's in-flight folder, and the victim then fails somewhere unrelated, naming a folder it never heard of.

The fix

Rather than rename 180 call sites — which would still leave Bloom's own production code writing to the shared temp directory while under test — redirect the whole process's temp directory before any fixture runs, with an assembly-wide [SetUpFixture]:

%TEMP%\BloomTests\<key>-p<pid>\

Every existing call site keeps its familiar name; the name is now scoped to this run. Path.GetTempPath() is defined in terms of TMP/TEMP, so this also covers the dozen tests that build temp paths by hand, and production code under test.

The <key> half comes from BLOOM_AGENT_BUILD_DIR when build/agent-dotnet.sh set it, so a temp folder can be matched to the build tree beside it; the pid is what guarantees uniqueness.

It lives in the global namespace deliberately: an NUnit [SetUpFixture] applies to its own namespace and those beneath it, and not quite all our fixtures are under BloomTests (there is one in Bloom.Api).

A failing run keeps its folder so the evidence survives — these bugs are precisely about temp folders appearing and disappearing, so deleting the scene of the crime would be perverse. Passing runs delete theirs; anything older than a day is cleared by a later run.

Effect on the suite

Measured on one developer machine, whose %TEMP% holds ~25,000 entries:

Tree	Total run	Passed	Failed	Aborted?
pristine master	1678	1603	65	yes
this branch	3046	3032	1	no

The suite ran to completion, and two consecutive runs gave identical results — so this is deterministic, not luck. The remaining failure, XMatterHelperTests.InsideBackCover_PaperAndDeviceXMatter_ShareOneCustomLayoutId, also fails on pristine master and is unrelated to temp; note that AGENTS.md calls a failing xmatter-locating test a real wrapper/Directory.Build.props regression, so it is worth chasing separately.

Tests

Two, asserting the invariant directly: that Path.GetTempPath() is this run's own folder and not the machine's, and that an ordinary fixed-name TemporaryFolder lands inside it.

Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16664

---

This change is 

---

Devin review

Devin review

[JohnThomson]

[Claude Opus 5 (1M context)] Consulted Devin on 2026-08-06 17:42 CDT up to commit 4ba357019edc6ddec694192d31b293740e14f3e7.

Four rounds, because each round's findings produced a fix commit, which starts a new review. Three things came out of it, each with its own resolved thread above:

• The two public NUnit hooks had no documentation comments, which the repo's guidance requires. Added.
• Nothing had actually verified the keep-on-failure behaviour, which depends on a corner of NUnit — whether a run's aggregated result is visible inside a global [SetUpFixture]'s teardown. Checked it directly: a run of one failing test leaves its folder behind, a run of one passing test creates and removes its own. That check also exposed a wrong sentence in the AGENTS.md note I had added, which is now corrected.
• Teardown deleted the run folder but left TMP/TEMP pointing at it, so anything running after the tests would have been handed a path inside a directory that no longer existed. Now restored before the delete.

Two notes for anyone reading the thread list. Devin appears to accumulate findings across rounds rather than recomputing them against the current diff: the final round re-reported all three of the above against commit 4ba35701, even though every one of them is fixed in that very commit. They were not posted again, since each already has a documented, resolved thread. Devin also left four informational observations each round, which are not mirrored.

CI: this repo runs no build or test checks on pull requests, so there was no CI signal. Greptile has stayed queued throughout without reviewing.

[JohnThomson]

[Claude Opus 5 (1M context)] Consulted Devin on 2026-08-07 09:32 CDT up to commit e9188cba75c9ffa7ff9aeb66ffc08b85b9e291eb.

Second preflight, after the developer asked for a warning when the run's temp folder cannot be
deleted. Devin found one further bug, now fixed and its thread resolved above:

• A run could inherit a dead run's scratch folder. The folder was created but never emptied,
  and its name is a label plus the process id — which Windows recycles — while folders from failing
  or crashed runs are deliberately kept for a day. So the two retention rules added in this branch
  were themselves widening the window for a run to start on top of stale files, which is the exact
  confusion the branch exists to remove. It now starts empty.

Its final round re-reported all four findings from this branch's history against e9188cba,
including that one — which is fixed in that very commit. As noted in the earlier log, Devin
accumulates findings across rounds rather than recomputing them, so nothing was posted twice; each
already has a documented, resolved thread.

Full suite at this commit: 3067 run, 3053 passed, 1 failed — XMatterHelperTests.InsideBackCover_PaperAndDeviceXMatter_ShareOneCustomLayoutId, which fails on pristine master too — and no abort.

One correction for the record: during this round MakePdf_BookNameIsNonAscii_OutputsPdf failed twice and I initially took it for a regression from this branch. It is not: it passed 5 of 5 subsequent runs in isolation and in two clean full runs. Both failures fell during a full-suite run that took 11 minutes rather than the usual 4-5, i.e. under heavy contention, and that test shells out to WebView2.

CI: this repo runs no build or test checks on pull requests. Greptile has stayed queued throughout without reviewing.

[andrew-polk]

@andrew-polk reviewed 3 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on JohnThomson).

---

src/BloomTests/TestTempDirectory.cs line 28 at r2 (raw file):

there is a fixture in the Bloom.Api namespace

Seems like we should fix that.

Maybe doesn't change this other than the comment, of course.

[JohnThomson]

@JohnThomson made 1 comment.
Reviewable status: 2 of 4 files reviewed, 4 unresolved discussions (waiting on andrew-polk).

---

src/BloomTests/TestTempDirectory.cs line 28 at r2 (raw file):

Previously, andrew-polk wrote…

there is a fixture in the Bloom.Api namespace

Seems like we should fix that.

Maybe doesn't change this other than the comment, of course.

Done.

[andrew-polk]

@andrew-polk reviewed 2 files and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 3 unresolved discussions.