ionide / ionide-vscode-fsharp

VS Code plugin for F# development
http://ionide.io
MIT License
849 stars 276 forks source link

Test explorer should use 'dotnet build' instead of 'msbuild /t:Build' to build the test project for test discovery #1967

Open Martin521 opened 7 months ago

Martin521 commented 7 months ago

Describe the bug

When working on the dotnet/fsharp repository in a dev container, the test explorer test discovery fails with a build error. Reason is

Steps to reproduce

Open the dotnet/fsharp directory in VS Code Dev Containers. Wait for the (misleading) error message Couldn't build test projects. Make sure you can build projects with 'dotnet build'.

Machine info

Additional context

As dotnet test is used for discovery and running of tests, I assume dotnet build is ok for building. Or is there a specific reason to use 'msbuild' directly? It would be nice to have test discovery work out of the box when opening dotnet/fsharp in a dev container. In any case, the error message should be changed to say ... Make sure you can build projects with 'msbuild /t:Build'.

TheAngryByrd commented 7 months ago

cc @farlee2121

farlee2121 commented 7 months ago

The test explorer uses msbuild because

As for the error message, the vast majority of the time dotnet build is going to boil down to the same result as msbuild but is much more likely to be in the user's path and thus easy for them to double check. I don't know for sure, but I'd guess dotnet build is a light wrapper around msbuild.

Considering tests were discovered for the fsharp repo in #1946, there might be something else going on here.

baronfel commented 7 months ago

dotnet build returns command success even if the build fails

This is surprising to me - we're getting the exit code from MSBuild here and should be flowing that up. Can you log an issue to dotnet/sdk with a repro?

farlee2121 commented 7 months ago

Maybe I need to revisit this and look more closely at the exit code, but I remember stumbling into this during my work. My notes say

Looks like dotnet build doesn't consider failed builds to be an error exit code. Nor does it write to stderr.

and I linked to this existing issue https://github.com/dotnet/sdk/issues/8481#issuecomment-314175413