Add ExecutableConditionAttribute to conditionally run tests based on tool availability

[Evangelink]

What

Adds a new public ExecutableConditionAttribute (in Microsoft.VisualStudio.TestTools.UnitTesting) that conditionally includes or excludes a test class or method based on whether a given executable/tool is available.

Two modes:

• Presence check — when no arguments are supplied, the condition only verifies that the executable is discoverable on PATH (and, on Windows, resolvable through PATHEXT). It does not run the executable. E.g. [ExecutableCondition("docker")].
• Run check — when arguments are supplied, the condition runs executable arguments and is met only when the process exits with code 0 within TimeoutSeconds (default 30s). Output is redirected/discarded and the process tree is terminated on timeout. E.g. [ExecutableCondition("docker", "version")].

Each distinct executable+arguments combination forms its own condition group, so multiple attributes with different commands AND together, while repeats of the same command OR together.

Details

• ConditionMode (Include/Exclude) overloads provided.
• Results are cached per command (availability doesn't change during a run).
• Cross-target: uses ProcessStartInfo.ArgumentList on modern .NET and the well-known PasteArguments quoting on netstandard/net462.
• New public API recorded in PublicAPI.Unshipped.txt; attribute is sealed and uses no init accessors.

Tests

Adds ExecutableConditionAttributeTests (TestFramework.ForTestingMSTest). All 21 cases pass on net8.0; project builds clean (0 warnings/0 errors) across all TFMs.

[Copilot]

Pull request overview

Adds a new MSTest condition attribute (ExecutableConditionAttribute) to gate test execution based on whether a tool is discoverable on PATH and (optionally) whether a command succeeds when executed, with result caching and multi-targeting support.

Changes:

• Introduces ExecutableConditionAttribute (public API) with presence-check and run-check modes plus configurable timeout.
• Records the new public surface in PublicAPI.Unshipped.txt.
• Adds unit tests for constructor behavior, grouping/message formatting, PATH probing, caching, and run/timeout behavior.

Show a summary per file

File	Description
src/TestFramework/TestFramework/Attributes/TestMethod/ExecutableConditionAttribute.cs	Implements the new conditional attribute, including PATH probing, optional process execution, timeout handling, and caching.
src/TestFramework/TestFramework/PublicAPI/PublicAPI.Unshipped.txt	Registers the new public API members for API tracking.
test/UnitTests/TestFramework.UnitTests/Attributes/ExecutableConditionAttributeTests.cs	Adds unit tests covering the new attribute’s behavior across scenarios.

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 7

[Evangelink]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary

The feature itself is well-structured: clean constructor hierarchy, good #if guarding across TFMs, correct async-drain pattern to avoid pipe-buffer deadlock, and solid process-kill handling. Two correctness bugs in the caching/grouping design need to be fixed before merge.

---

🔴 BLOCKING (2)

1 — GroupName / cache key is ambiguous (line 160)

The space separator used to join the executable and its arguments is the same character that can legally appear in an executable name. This means:

ExecutableConditionAttribute("foo bar")     // path-check for binary named "foo bar"
ExecutableConditionAttribute("foo", "bar")  // run "foo" with argument "bar"

Both produce GroupName = "ExecutableCondition:foo bar". Consequences:

• Cache collision: the static ResultCache is keyed by GroupName; whichever call runs first permanently determines the outcome for the other — despite them being fundamentally different operations (a File.Exists probe vs. a process run).
• Wrong OR-grouping: MSTest uses GroupName to cluster attributes for OR evaluation. These two attributes would be placed in the same group when they should be independent.

Suggested fix: use a mode prefix and a null-byte separator, e.g.:

$"ExecutableCondition:presence\0{Executable}"
$"ExecutableCondition:run\0{Executable}\0{string.Join("\0", _arguments)}"

2 — TimeoutSeconds is mutable but is not part of the cache key (line 151)

IsConditionMet reads ResultCache.GetOrAdd(GroupName, _ => Evaluate()), and Evaluate() uses TimeoutSeconds. But TimeoutSeconds is not reflected in GroupName (the cache key). Once a result is cached for a given executable+arguments combination, TimeoutSeconds is permanently ignored — on the same instance and on any other instance with the same command.

Concrete scenario on a slow CI machine where docker version takes ~8 s:

• [ExecutableCondition("docker", "version")] (30 s) runs first → caches true
• [ExecutableCondition("docker", "version", TimeoutSeconds = 5)] → same key → returns cached true, silently ignoring the 5 s constraint

The reverse is equally wrong: a 5 s timeout that times out poisons the cache for the 30 s use.

Suggested fix: split "cache key" from GroupName. Keep GroupName for OR-grouping (no timeout); add a private CacheKey property that appends TimeoutSeconds:

private string CacheKey => _arguments.Length == 0
    ? $"ExecutableCondition:presence\0{Executable}"
    : $"ExecutableCondition:run\0{Executable}\0{string.Join("\0", _arguments)}\0{TimeoutSeconds}";

public override bool IsConditionMet
    => ResultCache.GetOrAdd(CacheKey, _ => Evaluate());

---

🟡 Minor (1)

Redundant null-forgiving operator on line 316 — pathExt! after string.IsNullOrEmpty(pathExt) has already narrowed the type to non-null. Per the repo convention of trusting C# null annotations, the ! should be dropped.

[Evangelink]

🧪 Test quality grade — PR #9369

24 new tests graded across 1 file (ExecutableConditionAttributeTests); 22 A and 2 B. The suite is exceptionally well-named, tightly focused, and uses AwesomeAssertions throughout — a high bar for new attribute-coverage tests. The two B-grade tests are the cases that manipulate the process-wide PATH environment variable and write temporary files to disk; both use try/finally to clean up correctly, but the heavier arrange setup tips their structure sub-grade. Consider extracting the PATH-manipulation scaffold into a shared IDisposable helper to restore the clear AAA structure seen in the rest of the suite.

Δ	Test	Grade	Band	Notes
new	ExecutableConditionAttributeTests. IsConditionMet_ IsCachedPerExecutable	B	80–89	PATH mutation + file I/O in arrange with try/finally; complex setup tips structure to B — two assertions cleanly verify the caching contract.
new	ExecutableConditionAttributeTests. IsConditionMet_ WhenExecutableOnPath_ ReturnsTrue	B	80–89	PATH mutation and cross-platform file creation in arrange; correct try/finally cleanup but setup complexity tips structure to B.
new	ExecutableConditionAttributeTests. Arguments_ AreCopiedFromConstructorArray	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. Arguments_ DefaultsToEmpty	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. Arguments_ ReturnsProvidedValues	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. Constructor_ NullExecutable_ Throws	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. Constructor_ SetsCorrectMode	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. Constructor_ SingleArgument_ DefaultsToIncludeMode	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. Executable_ ReturnsProvidedValue	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. GroupName_ DiffersBetweenExecutables	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. GroupName_ DiffersByMode	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. GroupName_ IncludesArguments	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. GroupName_ IncludesExecutable	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. GroupName_ PresenceAndRunDoNotCollide	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. IgnoreMessage_ ExcludeMode_ ReturnsCorrectMessage	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. IgnoreMessage_ IncludeMode_ ReturnsCorrectMessage	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. IgnoreMessage_ WithArguments_ MentionsCommandSucceeds	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. IsConditionMet_ WhenCommandExceedsTimeout_ ReturnsFalse	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. IsConditionMet_ WhenCommandExitsNonZero_ ReturnsFalse	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. IsConditionMet_ WhenCommandExitsZero_ ReturnsTrue	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. IsConditionMet_ WhenExecutableMissing_ ReturnsFalse	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. IsConditionMet_ WhenExecutableMissingButArgumentsProvided_ ReturnsFalse	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. IsConditionMet_ WhenFullPathProvided_ ReturnsTrue	A	90–100	No issues found.
new	ExecutableConditionAttributeTests. TimeoutSeconds_ DefaultsTo30	A	90–100	No issues found.

_{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. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 244.3 AIC · ⌖ 13.8 AIC · ⊞ 45.6K · [◷]( · ◷)