Nest spreadsheet test temp folders so no test book sits in %TEMP% (BL-16661)

[JohnThomson]

SpreadsheetImporter.GetPageForLabel() looks for page templates in "the collection the book is in", which it computes as the book folder's parent. That is right in production — a book's parent folder really is its collection — but the spreadsheet tests built their book directly in %TEMP%, so the parent was the temp directory itself and Bloom walked every subfolder of it as a candidate book.

On a working machine that is thousands of folders belonging to other tests and other programs. If any of them was deleted between the enumeration and the inspection, FindBookHtmlInFolder threw and took out the whole fixture in OneTimeSetUp — naming an unrelated folder on the way out, and hitting a different set of tests each run.

The fix

New SpreadsheetTestFolders hands each fixture a folder of its own, named after the fixture, under a single root:

%TEMP%/BloomSpreadsheetTests/<FixtureName>/Book
                                          /Spreadsheet

Each fixture nests its book / spreadsheet / other folders inside that, so the folder the importer scans as "the collection" only ever holds that one fixture's folders. Teardown disposes just the root, which removes the rest. A side benefit: these tests now leave one entry in %TEMP% instead of three dozen.

The root is made with Directory.CreateDirectory rather than the TemporaryFolder(name) constructor, because that constructor first deletes any existing folder of the same name — which would throw away a sibling fixture's folders.

Three latent bugs this surfaced

Naming each folder after its own fixture (GetType().Name) fixed some collisions that were already there:

• SpreadsheetAudioTestsBase created folders named SpreadsheetImagesTests / SpreadsheetImagesTests_Book — the same names SpreadsheetImagesTests uses, so whichever ran second deleted the other's folder. That base is shared by five fixtures.
• Three license fixtures all used "SpreadsheetImportRemovingLicenseTests", and SpreadsheetImportDeleteExtraPagesTest used the name belonging to SpreadsheetImageAndTextImportToBookWithComplexLastPageTests.
• Both audio-import fixtures leaked _spreadsheetFolder (never disposed) and shared the name "other audio folder".

Test

Adds one guard test for the invariant the fix rests on: a book folder nested in one of these folders has, as its parent, a folder that is not %TEMP% and that contains nothing but our own folders.

All 391 BloomTests.Spreadsheet tests pass.

Both of the card's suggested fixes are here

The card offered two fixes and said "either or both". This PR now does both.

1. Keep test books out of %TEMP% (the root-cause fix, described above).

2. Make the scan itself tolerate a folder that vanishes. PageTemplatesApi.GetBooksInCollectionDirectories now skips a book folder that no longer exists rather than letting FindBookHtmlInFolder throw and abort the whole scan. This one protects the shipping product, not just the tests: when a user opens the Add Page dialog, Bloom lists the folders in every source collection and inspects each one, and a folder can be deleted in that instant by a sync client, an antivirus tool, or the user. It does not weaken the bl-291 warning that tells a user their book folder has gone missing — that check is on the save path, not the collection scan.

Its regression test reproduces the race deterministically rather than hoping to hit it: the scan is lazy, so the test takes one book from the enumerator, deletes the folder of whichever book it has not yet been handed (decided at run time, so the test does not depend on file-system enumeration order), and keeps enumerating. Without the guard it fails with the exact exception from the bug report — In FindBookHtmlInFolder('...'), the folder does not exist. (ref bl-291).

---

Devin review

---

This change is 

[JohnThomson]

[Claude Opus 5 (1M context)] Consulted Devin on 2026-08-06 16:13 CDT up to commit 26bba0d6ad4eac7bab314bddcf84d5f5cf5f9ee4.

Devin's review of this commit finished clean: no bugs and nothing flagged for investigation. It did leave three informational observations, none of which need a change — and two of them are useful confirmations of the assumptions this PR rests on:

• Fixture folder names are the only thing stopping one fixture from deleting another's folder. Devin checked every call site: the names are distinct, SpreadsheetAudioTestsBase.Setup really is called only once per subclass, the one static caller uses a name of its own, and NUnit fixture-level parallelism is not enabled in this assembly.
• Nesting the book under the fixture folder means the importer now looks at that fixture's other folders (ss, Dest, other audio folder, …) as candidate books. Harmless, because Bloom only looks for a top-level .htm/.html in each and none of them has one — the widget drag.html is in a subdirectory. Worth remembering if a future test drops an HTML file at the top of one of those folders.
• The %TEMP%/BloomSpreadsheetTests root is deliberately never deleted (deleting it would take out other fixtures' folders). The side effect is that if a fixture is renamed or removed, its old folder sits there until someone clears it by hand.

[JohnThomson]

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

Two rounds, because the first round's findings led to a fix commit. Across both, Devin raised no bugs and two things worth acting on, each with its own resolved thread above:

• The new regression test depended on the order the file system returns folders in — alphabetical on Windows, arbitrary on Linux — so it could have been flaky on Linux. Fixed: it now deletes whichever book the scan has not yet handed it.
• The PR description still said the production guard was not included, after the developer asked for it and I added it. Description rewritten to match the diff.

Devin also left four informational notes, which are observations rather than action items and are not mirrored as threads. One is worth repeating here because it is a genuine limitation rather than a nitpick: the Directory.Exists check narrows the race but cannot close it — a folder can still vanish in the instant between that check and FindBookHtmlInFolder opening it. That is inherent to a file system that other programs are also writing to; the guard turns a near-certainty on a busy machine into a very small window, and the remaining window would need a caught exception rather than a pre-check to close entirely.

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

[andrew-polk]

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