Skip to content

Quote boolean conditions in shipped Microsoft.TestPlatform.targets - #16404

Merged
Azat Mukhametshin (azat-msft) merged 2 commits into
microsoft:mainfrom
nohwnd:nohwnd-quote-boolean-conditions-in-shipped-targ
Aug 26, 2026
Merged

Quote boolean conditions in shipped Microsoft.TestPlatform.targets#16404
Azat Mukhametshin (azat-msft) merged 2 commits into
microsoft:mainfrom
nohwnd:nohwnd-quote-boolean-conditions-in-shipped-targ

Conversation

@nohwnd

Copy link
Copy Markdown
Member

dotnet test fails the build when a boolean property holds something that is not a boolean:

> dotnet test Repro.csproj -tl:on -p:VSTestNoBuild=1
  Repro net11.0 failed with 1 error(s)
    Microsoft.TestPlatform.targets(45,17): error MSB4100: Expected "$(VSTestNoBuild)" to
    evaluate to a boolean instead of "1", in condition "!$(VSTestNoBuild)".

Bare !$(X) and $(X) in a condition require the value to parse as a boolean, and 1 is not one. Three ways in, all reproduced against the shipped file:

command before
-p:VSTestNoBuild=1 MSB4100 at line 45, !$(VSTestNoBuild)
-p:VSTestNoBuild= MSB4100 at line 45, !$(VSTestNoBuild)
-p:VsTestUseMSBuildOutput=1 MSB4100 at line 33, !$(VsTestUseMSBuildOutput)

0/1 is a reasonable thing for a user to type, and this file already uses it for _MSBUILDTLENABLED two lines up, so copying the local idiom walks straight into it. The file ships in Microsoft.TestPlatform.Build and backs dotnet test, so this fails on the user machine, not on ours.

Compare against 'true' instead. MSBuild's == is boolean aware, so on, yes and TRUE still route the same way. Line 45 now reads '$(VSTestNoBuild)' != 'true', the same as the _VSTestConsole call site on line 81.

The two VSTestTaskAssemblyFile and VSTestConsolePath defaults are quoted for consistency with #16325, not because they were broken. MSBuild parses a condition from the raw string, so those already worked when the property was unset or held a path with spaces.

Verified by installing the built Microsoft.TestPlatform.targets into the SDK and running the same commands again. All three MSB4100 failures go away and the tests run; the other paths, including terminal logger on and off, VsTestUseMSBuildOutput=false and MSBUILDENSURESTDOUTFORTASKPROCESSES=1, produce the same output as before. MSBuildLoggerCanBeEnabledByBuildPropertyAndDoesNotEatSpecialChars, MSBuildLoggerCanBeDisabledByBuildProperty and MSBuildLoggerCanBeDisabledByEnvironmentVariableProperty pass with the new file, and passed before it as the control.

CopyTraceDataCollectorArtifacts in Microsoft.CodeCoverage.targets has the same problem, but #15794 is rewriting that target, so I left it there.

Fix #16396

🤖

Bare !$(VSTestNoBuild) and $(VsTestUseMSBuildOutput) make MSBuild require the
value to parse as a boolean, and it fails the build with MSB4100 when it does
not. dotnet test -p:VSTestNoBuild=1 is enough to hit it, and this file ships in
Microsoft.TestPlatform.Build, so it fails on the user machine.

Compare against 'true' instead. MSBuild's == is boolean aware, so on, yes and
TRUE keep routing the same way.

Fix microsoft#16396

🤖
Copilot AI lite review requested due to automatic review settings August 24, 2026 17:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request updates the shipped Microsoft.TestPlatform.targets (from Microsoft.TestPlatform.Build) to avoid MSBuild condition parsing failures when boolean-like properties are set to non-boolean values such as 0/1 or empty, which can break dotnet test on end-user machines.

Changes:

  • Quote property references in MSBuild Condition expressions to prevent malformed conditions when properties are unset/empty.
  • Replace bare boolean negation conditions (e.g., !$(...)) with explicit comparisons against 'true' to avoid MSB4100 when users pass 0/1-style values.
  • Quote the left-hand side of property default conditions for consistency and robustness when properties are unset.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Microsoft.TestPlatform.Build/Microsoft.TestPlatform.targets Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 26, 2026 14:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@azat-msft
Azat Mukhametshin (azat-msft) merged commit 3d105b5 into microsoft:main Aug 26, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[msbuild-quality] MSBuild file quality: unquoted conditions in shipped Microsoft.TestPlatform.targets

3 participants