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

[Bug]: dotnet test does not report failed test(s) information if terminal logger disabled #10358

Open
rainersigwald opened this issue Sep 24, 2024 · 8 comments

Comments

@rainersigwald
Copy link
Member


Issue moved from dotnet/msbuild#10682


From @martincostello on Saturday, September 21, 2024 3:42:19 PM

Issue Description

If the terminal logger is disabled (e.g. when running in CI in GitHub Actions) and a test fails, no visible output about the state of the test run is output to the console. Only an error code of 1 is returned: example

The user has to depend on an additional logger (such as GitHubActionsTestLogger) to see which tests failed and why, or has to re-run the tests locally with the terminal logger enabled to see what the failure is (which is problematic for CI-only failures).

Steps to Reproduce

To repro the exact failure above locally:

  1. Clone martincostello/openapi-extensions@0a12b92
  2. Edit build.ps1 to add --tl:off (code)
  3. Run build.ps1

Alternatively, run dotnet test --tl:off for a test suite using the latest .NET 9 RC2 build where one of the tests fails.

Expected Behavior

Some sort of message showing what tests failed is printed to the console. Ideally this would be similar to .NET 8's output modulo any improvements to the output of the total test counts etc.

Actual Behavior

Nothing is emitted that gives a hint as to whether tests are being run, or what the outcome was.

Analysis

A hunch tells me this has regressed at some point since preview 7.

Versions & Configurations

I can repro this with both 9.0.100-rc.1.24452.12 and 9.0.100-rc.2.24468.2 (a build from the last 24 hours).

@nohwnd
Copy link
Member

nohwnd commented Sep 24, 2024

oh god that's bad. @martincostello I am looking into this.

@nohwnd
Copy link
Member

nohwnd commented Sep 24, 2024

In the old flow where you specify -p:VSTestUseMSBuildOutput=false to disable the new output, we parse it out from that execution parameters inside of dotnet test command (so we process it way before MSBuild),

https://github.com/dotnet/sdk/blob/main/src/Cli/dotnet/commands/dotnet-test/Program.cs#L61

And we set: MSBUILDENSURESTDOUTFORTASKPROCESSES=1, before calling msbuild. That gets picked up here:

https://github.com/dotnet/msbuild/blob/main/src/MSBuild/XMake.cs#L251

And it sets up the child processes in a special way so vstest can write to standard output and show errors.

But with the new approach of detecting -tl:false,

https://github.com/dotnet/msbuild/blob/main/src/MSBuild/XMake.cs#L2529-L2533

we don't set
MSBUILDENSURESTDOUTFORTASKPROCESSES env variable.
This makes all the vstest.console output to be lost.


Looking at the logic of detecting if TL should be enabled and it is pretty complex. So the only option I see is that we change MSBuild to be aware of vstest, by moving the check for -tl:false in xmake above the detection of MSBUILDENSURESTDOUTFORTASKPROCESSES, and check if targets include VSTest. and set MSBUILDENSURESTDOUTFORTASKPROCESSES=1.

But that seems dirty, do you see any other options @rainersigwald or @MichalPavlik ?

@rainersigwald
Copy link
Member Author

I wouldn't want to regress the performance of folks who opt out of -tl by setting MSBUILDENSURESTDOUTFORTASKPROCESSES all the time when opted out. @baronfel do you see an easy way forward?

@baronfel
Copy link
Member

Is this yet another case where it would be useful if the CLI knew what the TL status was? Do we need to duplicate/capture that logic in the SDK at this point?

@rainersigwald
Copy link
Member Author

It seems like it.

@nohwnd
Copy link
Member

nohwnd commented Sep 24, 2024

Yes, if we’d know in CLI then we can just set MSBUILDENSURESTDOUTFORTASKPROCESSES for the old flow as we did until now.

@nohwnd
Copy link
Member

nohwnd commented Sep 26, 2024

@baronfel does this mean you will be exporting the functionality from MSBuild so we can use it in sdk? :)

@nohwnd
Copy link
Member

nohwnd commented Oct 9, 2024

fixed in dotnet/sdk#43871

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

No branches or pull requests

3 participants