Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

Suppress msb4181 from vstest #4941

Merged

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Oct 8, 2020

Description

Set AllowFailureWithoutError to prevent the error MSB4181 from being output.

MSB4181: The "Microsoft.TestPlatform.Build.Tasks.VSTestTask" task returned false but did not log an error.

This error is caused by VSTest outputting errors directly to the screen instead of using MSBuild, and is blocking some builds from finishing. The same fix was done in net5.0.

There is no workaround, other than sticking with 3.1.302 SDK.

Those are the changes included in the update:

microsoft/vstest@4f345e9
microsoft/vstest@d8b5912

The option is set to "false" because the version of MSBuild that is being used has the option implemented backwards. This issue was reported to MSBuild and will be fixed in 16.9.

Customer Impact

Anyone who updated their 3.1 SDK from "3.1.302" to a newer version is impacted by this. Any failing test will make the VSTest MSBuild task fail and the build will fail with a confusing message. This makes the CI failures harder to diagnose, because the first error that you see is the MSBuild error, instead of the "x tests failed" error.

Regression?

There was a change of behavior in MSBuild https://github.com/dotnet/msbuild/blob/ca44138662e3aa90eb9305dd31d906ef02e962cb/src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs#L940-L958 that started showing this error for all tasks that return false but write their errors to screen in non standard way. VSTest is one of them, we write directly to screen because we want to have colors in our output to easily distinguish passed and failed tests.

Risk (see taxonomy)

Very low. The change is limited to dotnet test. Tests can finish running with or without this change in place. Only the reporting of failed test results is affected.

Link the PR to the original issue and/or the PR to master.

Fix dotnet/sdk#13431

Packaging impact? (if a libraries change)

I am not aware of any.

@nohwnd
Copy link
Member Author

nohwnd commented Oct 13, 2020

Added the label on behalf of @ManishJayaswal , you can see he set it on the linked issue, but does not have permissions here.

@mwpowellhtx
Copy link

Not sure what is being asked here, but this is not a fix of a regression.

Re: regression, of what? In the runtime? Probably... But I do not know why it would be any different in the 3.1.402 runtime configuration versus any other.

Very low. Tests can finish running with our without this change in place.

Vis-a-vis "risk", I'm not sure how you can make this claim, because I, too, have encountered the same issue in my BumpAssemblyVersions deployments.

I am not aware of any.

Neither are we (i.e. aware of any), but evidently SOMETHING did change, or we would not be bumping into this error.

My only advice here would be to exercise caution. Is this a bandaid for the toolset? Or is it addressing a root cause.

I will file a separate issue focused from the perspective of my BumpAssemblyVersions tooling, but for now, this is another data point. It does seem like the tooling is trying to run, but for whatever reason the MSBuild runtime is failing to connect the dots with my tooling. Again, in a WindowsDesktop Sdk configuration, targeting netcoreapp3.1.

1>------ Rebuild All started: Project: bavtest, Configuration: Debug Any CPU ------
1>path\to\BumpAssemblyVersions\feature-win-desktop\docs\bavtest\packages\bumpassemblyversions\1.6.1.3\build\BumpAssemblyVersions.targets(14,9): warning : No descriptors were provided in order to bump.
1>path\to\BumpAssemblyVersions\feature-win-desktop\docs\bavtest\packages\bumpassemblyversions\1.6.1.3\build\BumpAssemblyVersions.targets(14,9): error MSB4181: The "BumpVersion" task returned false but did not log an error.
1>Done building project "bavtest.csproj" -- FAILED.
========== Rebuild All: 0 succeeded, 1 failed, 0 skipped ==========

@marcpopMSFT
Copy link
Member

@nohwnd checking on old PRs. This was marked servicing approved. Should this have been merged? Is it still needed?

@nohwnd
Copy link
Member Author

nohwnd commented Jan 21, 2021

@marcpopMSFT yes this should have been merged. I just checked the latest 3.1.406 sdk and it still has the error. If it can be merged now, please merge.

image

@marcpopMSFT
Copy link
Member

Checked with a couple of folks and we still have time and approval for Feb releases. I'm going to merge.

@marcpopMSFT marcpopMSFT merged commit 5649c42 into dotnet:release/3.1.4xx Jan 21, 2021
@nohwnd
Copy link
Member Author

nohwnd commented Jan 22, 2021

Thank you 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants