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

Add the $(VSTestUseConsole) property to the dotnet test command #15376

Closed
wants to merge 1 commit into from

Conversation

mcartoixa
Copy link

This PR is a prerequisite for microsoft/vstest#2702. It adds the $(VSTestUseConsole) property to the MSBuild process launched by dotnet test. As a result this command will behave like it does today (cf. microsoft/vstest#680): colorized output, no MSBuild logs.

microsoft/vstest#2702 adds the ability to use MSBuild directly if logs (or node reuse) are more important than colorization.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@wli3
Copy link

wli3 commented Jan 21, 2021

@nohwnd could you review this?

@nohwnd
Copy link
Member

nohwnd commented Jan 21, 2021

There is a long way to merging microsoft/vstest#2702 if we ever decide to do that. Adding this option before we decide and implement the change in VSTest is imho unnecessary. You can specify it in command line as -p directly, or add it as a default value in the msbuild VSTest task to test it out.

I am voting for closing this issue without merging.

@mcartoixa
Copy link
Author

Sorry if I was not clear but as designed the statuses of these 2 PRs are linked IMHO. Either merge both or neither of them.
The description of microsoft/vstest#2702 is more complete but it still may need further explanations that I would be more than willing to provide.

@nohwnd
Copy link
Member

nohwnd commented Jan 21, 2021

To me there is no need to keep this PR open until (and if) we make the change in VSTest because the new behavior won't be available till then and this PR will just add to the noise in this repo. This is a 1 line change, that is easy to provide via command line while we refine the behavior on the vstest side.

I don't want to discourage you or anything, we are already internally discussing what to do about your vstest PR, but this PR seems premature.

It's up to the maintainers of this repo, but I'd vote for closing this without merge.

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.

None yet

4 participants