Add per-test temporary directory (TestContext.TestTempDirectory) — RFC 021

[Evangelink]

Summary

Adds a per-test unique temporary directory to MSTest, exposed as a new additive TestContext.TestTempDirectory property, and the accompanying RFC 021. This closes a genuine gap: every other major test ecosystem ships a per-test temp directory (Go t.TempDir(), pytest tmp_path, JUnit 5 @TempDir, Rust tempfile::TempDir, Playwright outputDir) and .NET has none. MSTest V1 actually gave each test a unique TestResultsDirectory; V2 collapsed it — reported as #502 and closed won't-fix. In hindsight, that was this feature.

Without it, tests that write to disk hardcode shared paths (which race under parallel execution and force [DoNotParallelize]) or hand-roll a GetTempPath() + Guid + IDisposable helper — copy-pasted across the ecosystem, including this very repo's TempDirectory.cs.

Behavior

• New property: public virtual string? TestContext.TestTempDirectory — additive; no existing directory property changes semantics.
• Lazy: created on first access, so tests that never touch it pay nothing.
• Unique per test and per [DataRow]/[DynamicData] case (each case runs on its own TestContext). Uniqueness comes from a full 128-bit GUID suffix, so it holds even for many cases of the same method running concurrently.
• Located under TestResultsDirectory (discoverable next to other run output). The readable directory name is not a fixed short string — its length is computed adaptively from how deep the base path already is (capped at 50 characters), always reserving a working headroom of 80 characters for the files the test itself writes so ordinary relative writes inside it will not overflow MAX_PATH.
• Fallback to Path.GetTempPath() happens when the results directory is unavailable, when it is so deep that even a minimal readable name cannot preserve the 80-char headroom, or when the chosen base directory cannot be written to (e.g. a read-only output directory). Note TestResultsDirectory defaults to the test assembly's output (bin) directory when no results directory is configured, so by default the per-test directory is created there.
• Long paths: the feature targets the classic 260-character MAX_PATH and does not rely on long-path opt-in (LongPathsEnabled / the \\?\ prefix), which is not guaranteed to be enabled and is often not honored by external tools that E2E tests shell out to.
• Cleanup: deleted on pass, retained on failure (a failed test's artifacts are what you want to inspect). Env-var escape hatch MSTEST_TEST_TEMP_DIRECTORY_RETAIN=1 retains everything. Best-effort in Dispose: never throws (leaked handles / AV locks are swallowed and traced), and runs after the test so it can't extend or fail it.

See docs/RFCs/021-Per-Test-Temporary-Directory.md for the full motivation, all 9 design questions answered with rationale, an explicit comparison table against the existing five directory properties, and the relationship to RFC 020 ([ResourceLock]): eliminate the sharing (this) before you coordinate access to it (ResourceLock).

Decisions needing sign-off ⚠️

These are called out in the RFC as **(sign-off)** and should be ratified, not assumed settled:

1. Name — TestTempDirectory (chosen to be unambiguous vs the existing five directory properties).
2. Location — under TestResultsDirectory, with the adaptive-budget + temp-path fallback described above (vs always Path.GetTempPath()).
3. Cleanup policy — delete-on-pass / retain-on-failure by default, plus the MSTEST_TEST_TEMP_DIRECTORY_RETAIN escape hatch (vs always-delete or a runsettings knob, which is deferred).

Secondary settled-but-tunable values: readable-name cap 50, floor 8, unique suffix 32 hex chars (128-bit GUID), reserved headroom 80; cleanup-failure is trace-only (no test warning).

Non-goals (in RFC)

• Does not redirect Environment.CurrentDirectory (process-global; corrupts parallel tests).
• No class/assembly-level shared temp directory, no keep-last-N-runs — noted as future work.

Tests

• Unit tests (MSTestAdapter.PlatformServices.UnitTests): lazy creation, uniqueness across contexts and across data-driven clones, delete-on-pass, retain-on-failure, env-var retain, cleanup swallows errors, GetTempPath() fallback (unavailable / too-deep / unwritable base), post-dispose no-create race, folded-row cleanup-failure retention, long/invalid-name sanitization bounds, and a non-BMP / surrogate-pair name that truncation must not split.
• Acceptance test (MSTest.Acceptance.IntegrationTests): end-to-end proof of uniqueness across concurrently running tests, uniqueness across [DataRow] cases, lazy creation (no directory for a test that never accesses it), cleanup on pass, retention on failure, and retention of folded [DataRow] rows whose [TestCleanup] fails.

Validated with .\build.cmd -pack -test (green across all TFMs) and the acceptance tests passing.

Notes

New public API added to the four relevant PublicAPI.Unshipped.txt files; no init accessor. This branch is independent of the sibling [ResourceLock] branch — RFC 020 was read for alignment only and is not modified.

[Copilot]

Pull request overview

Adds RFC 021’s lazy, per-test temporary directory API to MSTest, including lifecycle management, API tracking, and automated coverage.

Changes:

• Adds TestContext.TestTempDirectory.
• Creates unique directories and applies outcome-based cleanup.
• Adds unit, acceptance, and RFC documentation coverage.

Show a summary per file

File	Description
src/TestFramework/TestFramework.Extensions/TestContext.cs	Declares and documents the public property.
src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs	Implements creation, naming, retention, and cleanup.
src/Adapter/MSTestAdapter.PlatformServices/InternalAPI/InternalAPI.Unshipped.txt	Tracks the internal override.
src/TestFramework/TestFramework.Extensions/PublicAPI/net462/PublicAPI.Unshipped.txt	Tracks the net462 API.
src/TestFramework/TestFramework.Extensions/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt	Tracks the netstandard2.0 API.
src/TestFramework/TestFramework.Extensions/PublicAPI/net8.0/PublicAPI.Unshipped.txt	Tracks the net8.0 API.
src/TestFramework/TestFramework.Extensions/PublicAPI/net9.0/PublicAPI.Unshipped.txt	Tracks the net9.0 API.
test/UnitTests/MSTestAdapter.PlatformServices.UnitTests/Services/TestContextImplementationTests.cs	Covers creation, uniqueness, cleanup, and retention.
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/PerTestTempDirectoryTests.cs	Adds end-to-end behavior validation.
docs/RFCs/021-Per-Test-Temporary-Directory.md	Documents motivation, design, and compatibility.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 4
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (2)

src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs:615

• Path.GetInvalidFileNameChars() is OS-specific; on Linux/macOS it does not include :, *, ?, \\, <, >, or |. Those characters therefore survive sanitization, so the new cross-platform unit test at lines 765-767 fails and generated names are not portable as documented. Add the stable Windows-invalid/control-character set in addition to the platform-provided set.

            if (char.IsWhiteSpace(c) || Array.IndexOf(invalidChars, c) >= 0)

src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs:707

• This retention decision can see a stale passing outcome. TestMethodInfo.InvokeAsync calls SetOutcome(result.Outcome) before RunTestCleanupMethodAsync, while that cleanup method can subsequently change a passing result to Failed/Inconclusive without updating the context. Consequently, if the test body passes but [TestCleanup], Dispose, or a global cleanup fails, this code deletes the directory even though the final test result is non-passing, violating the advertised retain-on-failure contract. Propagate the post-cleanup/final outcome to this context before disposal and cover that lifecycle case.

        if (_outcome != UnitTestOutcome.Passed)

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (2)

src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs:592

• This budget allows the returned path itself to be 180 characters, so appending 80 characters produces a 260-character visible path. Classic Win32 MAX_PATH includes the terminating NUL, leaving only 259 visible characters; the documented 80-character headroom therefore fails at the boundary. Reserve one additional character for the terminator.

        => WindowsMaxPath

src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs:581

• The adaptive calculation budgets for a 12-character suffix, but this fallback substitutes a 32-character GUID. With a deep base path, this rare branch consumes 20 characters of the promised 80-character headroom and can cross the classic path limit. Recompute the fallback budget or place the full-GUID fallback under a sufficiently short root.

        string fallback = Path.Combine(baseDirectory, Guid.NewGuid().ToString("N"));
        Directory.CreateDirectory(fallback);

• Files reviewed: 12/12 changed files
• Comments generated: 3
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (1)

src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs:562

• The 80-character headroom is not preserved when the Windows system temp path is itself long (for example, via a custom TEMP). After falling back, available may still be negative, but clamping the readable budget to zero only removes the name; the 12-character suffix remains. A temp root longer than 167 characters therefore returns a directory path longer than 180 characters, contradicting the public XML/RFC guarantee, and a still-deeper root can make the final GUID fallback throw under classic MAX_PATH. Validate the minimum suffix-only path against the budget and either select a genuinely shorter writable root or qualify the documented guarantee.

                baseDirectory = Path.GetTempPath();
                baseIsTemp = true;
                available = ComputeReadableNameBudget(baseDirectory);
            }

            nameBudget = available < 0 ? 0 : Math.Min(available, TestTempDirectoryNameMaxLength);

• Files reviewed: 12/12 changed files
• Comments generated: 2
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (1)

src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs:606

• Clamping a negative budget to zero does not preserve the documented 80-character headroom when the system temp root itself is deep. Windows allows TMP/TEMP to point to an arbitrarily long path; once the results path falls back there, TryCreateTestTempDirectoryUnder can still succeed on long-path-enabled .NET and return <deep-temp>\<guid> longer than 180 characters, even though this API explicitly promises compatibility with classic MAX_PATH consumers. Validate that the GUID-only candidate fits the budget too; otherwise use a genuinely shorter base or fail/document that the guarantee cannot be met. The final fallback at lines 629-633 needs the same check.

                available = ComputeReadableNameBudget(baseDirectory);
            }

            nameBudget = available < 0 ? 0 : Math.Min(available, TestTempDirectoryNameMaxLength);

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (1)

src/Adapter/MSTestAdapter.PlatformServices/Services/TestContextImplementation.cs:628

• If the configured Windows temp root is itself deep, available is still negative after the results-directory fallback. Clamping only the readable prefix to zero cannot recover the promised 80-character headroom because the 32-character GUID suffix is irreducible; on modern .NET with long paths enabled, creation can succeed and return a path that classic-MAX_PATH external tools cannot use. Validate the complete temp candidate against the headroom budget and either choose a genuinely shorter base or fail explicitly instead of returning a path that violates the API guarantee.

            nameBudget = available < 0 ? 0 : Math.Min(available, TestTempDirectoryNameMaxLength);

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Medium

[Copilot]

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 2
• Review effort level: Medium

[Copilot]

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 0 new
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #10233

24 tests graded across 2 files (4 integration, 20 unit). 23 earn A and 1 earns B. The suite is exceptionally well-written: every test follows a clear Arrange-Act-Assert pattern, assertions are concrete and multi-faceted, and the mutation story is nearly perfect. The single B is TestTempDirectoryShouldFallBackToTempPathWhenResultsDirectoryTooDeep, where the non-Windows code path only checks that the directory exists — the parent-directory assertion is gated inside if (onWindows), so a wrong-location mutation survives on Linux/macOS.

Grade	Test	Mutation	Notes	How to improve
B (80–89)	new TestContextImplementationTests. TestTempDirectoryShouldFallBackToTempPath WhenResultsDirectoryTooDeep	2/3 killed	Non-Windows path only asserts the directory exists; wrong-parent mutation survives on Linux/macOS.	Add Path.GetDirectoryName(tempDirectory).Should().Be(tempRoot) outside the if (onWindows) block (the fallback fires on all platforms).
A (90–100)	new PerTestTempDirectoryTests. TestTempDirectory_ IsCreatedLazily	3/4 killed	Clear integration contract; sibling-directory check covers both accessor and non-accessor cases.	—
A (90–100)	new PerTestTempDirectoryTests. TestTempDirectory_ IsRetained_ OnPass_ WhenResultFileRegisteredUnderIt	1/1 killed	Concrete File.Exists assertion catches any delete-on-pass regression.	—
A (90–100)	new PerTestTempDirectoryTests. TestTempDirectory_ IsRetained_ WhenDataRowCleanupFails	2/2 killed	Count check and per-row Directory.Exists cover both rows of the cleanup-failure scenario.	—
A (90–100)	new PerTestTempDirectoryTests. TestTempDirectory_ IsUnique_ CleansUpOnPass_ And_ RetainsOnFailure	4/4 killed	Five distinct assertions cover uniqueness, data-row uniqueness, pass cleanup, fail retention, and artifact survival.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryCleanupShouldSwallowErrors	1/1 killed	NotThrow correctly verifies the swallow contract; locked file exercises the error path.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldBeDeletedOnPass	1/1 killed	BeFalse on the directory catches any retain-on-pass mutation.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldBeDeletedOnPassWhen OnlyEarlierRetryAttemptRegisteredResultFile	1/1 killed	GetResultFiles() call pattern mimics the runtime's per-attempt drain; BeFalse catches sticky-flag mutation.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldBeDeletedOnPassWhen ResultFileIsOutsideIt	1/1 killed	Outside file does not trigger retention; BeFalse on the temp dir is the right discriminating assertion.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldBeRetainedOnFailure	1/1 killed	BeTrue after Failed outcome catches any delete-on-failure regression.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldBeRetainedOnPassWhen ResultFileRegisteredUnderIt	2/2 killed	Both dir and file BeTrue guard against premature deletion.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldBeRetainedWhen EnvironmentVariableIsSet	1/1 killed	Env-var is set, reset in finally; BeTrue confirms force-retain path works.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldBeRetainedWhen OutcomeChangesToFailedAfterCreation	1/1 killed	Pre-cleanup Passed + post-cleanup Failed; BeTrue catches the regression where the first outcome wins.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldBeUniqueAcrossContexts	2/2 killed	NotBe plus two BeTrue verify distinctness and real creation.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldBeUniqueAcross DataDrivenClones	2/2 killed	Clone path uses CloneForDataDrivenIteration; NotBe + BeTrue cover the data-driven uniqueness contract.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldCreateDirectoryUnder ResultsDirectoryOnFirstAccess	4/4 killed	Checks non-null, directory exists, parent equals results dir, and name prefix — all four meaningful mutations killed.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldFallBackToTempPath WhenNoResultsDirectory	4/4 killed	Existence, parent root, and BeFalse after dispose collectively protect all branches of the no-results path.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldFallBackToTempPath WhenResultsDirectoryIsNotWritable	2/2 killed	File-as-directory triggers the IOException fallback; parent assertion pins the fallback location.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldNotCreateDirectory AfterDispose	2/2 killed	BeNull and BeEmpty together verify both the return value and the side-effect after disposal.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldNotCreateDirectory WhenNeverAccessed	1/1 killed	BeEmpty on the results directory proves lazy creation — no access, no directory.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldNotSplitSurrogate PairsWhenTruncatingName	2/2 killed	Loop explicitly validates each char position; any off-by-one in the surrogate guard would trip BeTrue.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldReturnNullFor NonTestContexts	2/2 killed	BeNull and BeEmpty verify both the null return and absence of side-effects for fixture contexts.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldReturnSamePath OnRepeatedAccess	1/1 killed	second.Should().Be(first) catches any mutation that returns a fresh path per call.	—
A (90–100)	new TestContextImplementationTests. TestTempDirectoryShouldSanitizeAndBound LongNameWithInvalidCharacters	5/5 killed	Per-character NotContain checks plus length bound cover all branches of sanitization.	—

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · sonnet46 108.6 AIC · ⌖ 5.18 AIC · ⊞ 10.3K · [◷]( · ◷)