Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/9.0.1xx-preview1] Fix VSTest MSBuild and Terminal Logger integration #38371

Merged

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Jan 30, 2024

VSTest added new MSBuild integration and made it the default because it wants to be consistent with terminal logger usage in commands like dotnet build. There were 3 issues discovered during dogfooding:

  • special characters are mangled, because encoding is wrong
  • outputting the text "error" to output will fail the build, because msbuild parses it out and reports as error
  • it is hard to disable the new approach when wrapping dotnet test in msbuild (such as source build does it)

(summary and links to each issue and PR are here : microsoft/vstest#4843)

This PR fixes all three issues.

Risk of this fix is small, we are fixing isolated part, which has an opt-out flag. And the fixes are also simple. Setting utf8 encoding that we enforce in vstest.console anyway. And writing out message using a different msbuild api, that avoids re-parsing it.

Full diff between commits in vstest: microsoft/vstest@53df73d...d617595

dotnet-maestro bot and others added 2 commits January 30, 2024 15:07
…240130.1

Microsoft.NET.Test.Sdk , Microsoft.TestPlatform.Build , Microsoft.TestPlatform.CLI
 From Version 17.10.0-preview-24078-03 -> To Version 17.10.0-preview-24080-01
@nohwnd nohwnd requested a review from a team as a code owner January 30, 2024 14:22
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-DotNet Test untriaged Request triage from a team member labels Jan 30, 2024
@nohwnd nohwnd changed the title Fix VSTest MSBuild and Terminal Logger integration [release/9.0.1xxx-preview1] Fix VSTest MSBuild and Terminal Logger integration Jan 30, 2024
@nohwnd nohwnd changed the title [release/9.0.1xxx-preview1] Fix VSTest MSBuild and Terminal Logger integration [release/9.0.1xx-preview1] Fix VSTest MSBuild and Terminal Logger integration Jan 30, 2024
@nohwnd
Copy link
Member Author

nohwnd commented Jan 30, 2024

@marcpopMSFT I see that some PRs are still getting into Preview1, if it is still possible to merge then please merge.

I am also unsure what to do about the Darwin fail. That does not look like my mistake.

@baronfel
Copy link
Member

@nohwnd that looks like a transient git failure, we'll rerun

@baronfel
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@marcpopMSFT
Copy link
Member

Runtime is taking some additional changes so going ahead and merging this. @nohwnd you'll need a separate PR into main/8.0.3xx as we don't flow automatically

@marcpopMSFT marcpopMSFT merged commit 029e25e into dotnet:release/9.0.1xx-preview1 Jan 30, 2024
16 checks passed
@nohwnd
Copy link
Member Author

nohwnd commented Jan 31, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-DotNet Test untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants