ionide-vscode-fsharp icon indicating copy to clipboard operation
ionide-vscode-fsharp copied to clipboard

Setting TestingPlatformDotnetTestSupport makes tests non discoverable

Open OnurGumus opened this issue 9 months ago • 15 comments

Describe the bug

There is a new post from microsoft dev blogs

They introduced TestingPlatformDotnetTestSupport switch in the project file. Once you set that true, tests aren't discoverable by ionide.

     <TestingPlatformDotnetTestSupport>true</TestingPlatformDotnetTestSupport>

To be fair I am not entirely sure what this new switch brings to table.

Steps to reproduce

Create any test project then <TestingPlatformDotnetTestSupport>true</TestingPlatformDotnetTestSupport>

Ionide will complain no tests are discoverable.

Expected behaviour

Ionide should discover the tests

Machine info

  • MacOS 15
  • .NET SDK version:9
  • Ionide version: 7.25

Additional context

Add any other context about the problem here.

OnurGumus avatar Mar 01 '25 17:03 OnurGumus

cc @farlee2121

TheAngryByrd avatar Mar 02 '25 20:03 TheAngryByrd

I tested this, and found some significant problems.

First, Microsoft.Testing.Platform doesn't respect <GenerateProgramFile>false</GenerateProgramFile>.

This means that using versions of YoloDev.Expecto.TestSdk which depend on Microsoft.Testing.Platform can't define their own main method (and thus can't be run as a normal expecto project). This is true even if EnableExpectoTestingPlatformIntegration and TestingPlatformDotnetTestSupport aren't enabled.

Second, Microsoft.Testing.Platform doesn't respect dotnet test flags like --logger and --list-tests. This makes it incompatible with the way the Ionide test explorer currently works, which relies on --logger to discover tests and to report test results.

I confirmed that the --logger and --list-tests support is also missing from XUnit, not just YoloDev/Expecto.

In short, Microsoft.Testing.Platform won't work with Ionide short of rewriting test and result discovery or Microsoft.Testing.Platform better respecting dotnet test parameters.

farlee2121 avatar Mar 03 '25 18:03 farlee2121

Thanks for looking into this @farlee2121!

TheAngryByrd avatar Mar 03 '25 19:03 TheAngryByrd

Hi @OnurGumus, @farlee2121, and @TheAngryByrd

Let me clarify few points.


First, Microsoft.Testing.Platform doesn't respect <GenerateProgramFile>false</GenerateProgramFile>.

GenerateProgramFile is a VSTest-specific property. If you want to disable the entry point generation of Microsoft.Testing.Platform, you should use GenerateTestingPlatformEntryPoint instead.


Second, Microsoft.Testing.Platform doesn't respect dotnet test flags like --logger and --list-tests.

There are three points to make here:

  1. dotnet test original design was VSTest-oriented. That means all the dotnet test options are directly forwarded to VSTest. What TestingPlatformDotnetTestSupport does is that it tries to kinda plug Microsoft.Testing.Platform into the existing infrastructure that allows dotnet test to run. Per the current infrastructure of dotnet test, the only thing that Microsoft.Testing.Platform can see is whatever comes after --. So, you can do things like dotnet test -- --list-tests.

  2. In .NET 10 SDK, we are bringing better support for dotnet test that's MTP-oriented. The new experience will be opt-in via a new configuration file, called dotnet.config. Example file:

    [dotnet.test:runner]
    name = "Microsoft.Testing.Platform"
    
  3. For --logger, the value you pass in there is VSTest-specific, and thus won't work for Microsoft.Testing.Platform. If you do --logger "mylogger", there will be an implementation of a logger with friendly name mylogger somewhere, and the implementation will be relying on VSTest APIs. So, by-design it's not compatible with Microsoft.Testing.Platform. Such logger should be written as a Microsoft.Testing.Platform extension. Note that VSTest kinda mixed the concept of loggers and reporters, which are separate now. It's more likely that what you need is a reporter. We have a sample of how to write your own reporter. We will be more than happy to help you with any further questions regarding Microsoft.Testing.Platform. Please, don't hesitate to reach out via issues/discussions on microsoft/testfx, and I hope this comment clarifies the situation for you. NOTE: For trx, we already have it as an extension, https://learn.microsoft.com/dotnet/core/testing/microsoft-testing-platform-extensions-test-reports

Youssef1313 avatar Mar 20 '25 08:03 Youssef1313

I've started digging into this. I see how to pass arguments to to Microsoft.Testing.Platform. I also can install the TrxReport extension package and get a report.

I'm not seeing a clear way that I can uniformly collect test results without requiring the user to install a package or modify their program.

It looks like Microsoft.Testing.Platform.MSBuild and Microsoft.Testing.Platform are the only packages I can expect to come standard.

--results-directory comes standard with these packages, but no result formats to match. All the report formats seem to come from extensions and just setting --results-directory alone doesn't result in a report file.

The --diagnostic log seems like it could probably provide the right info, but would be a pain to parse.

Alternatively, I also haven't found a way to simply register an extension with the test runner without modifying the tested project. I.e. I tried adding Microsoft.Testing.Extensions.TrxReport dynamically. Digging into the source, TrxReport appears to require a build-time hook to invoke registration code and is not discoverable dynamically.

@Youssef1313 Am I missing something? Is there an intended way for test explorers to collect test results?

farlee2121 avatar Apr 06 '25 20:04 farlee2121

@farlee2121 It's curious IMO if a test explorer is currently relying on TRX (even for VSTest).

For the case of MTP specifically, I think test explorers are expected to launch the executable in server mode and communicate with it via JsonRpc. cc @drognanar

If you really need TRX, users are forced to install the TrxReport package in their projects.

Youssef1313 avatar Apr 07 '25 05:04 Youssef1313

@Youssef1313 The Ionide test explorer was built over dotnet test and trx files.

I previously looked into Microsoft.TestPlatform.TranslationLayer, which seems to be the VSTest approach to integration, but got pulled away before I understood it well enough to migrate. It sounds like this JsonRPC approach might be similar.

Could you provide links to documentation on the server mode and JsonRpc approach you mentioned?

farlee2121 avatar Apr 07 '25 15:04 farlee2121

We have documentation here, but parts of it may not be very up-to-date. Maybe the implementation can help along with the doc, which here

Youssef1313 avatar Apr 07 '25 16:04 Youssef1313

I got samples of both VSTest and MTP running over their JSON protocols Here's the repo

In short, VSTest still seems like the way to go.

VSTest can run both VSTest-based projects and any projects with dual MTP/VSTest support, which seems like everything but TUnit. I tested Expecto, xUnit, NUnit, and MSTest. It also has an official nuget package for the wrapper around the JSON protocol

MTP didn't seem like it could run VSTest-based projects. Granted, I might just not have figured out how. I had to flail at MTP a fair bit to get it running. MTP also doesn't have a public wrapper for its JSON protocol.

A switch to VSTest would get us

  • Consistent discovery behavior with Visual Studio, dotnet test, and Rider
  • We can remove all our code analysis for locating tests, instead relying on the code location built into each library's test adapter
    • We should also get code locations for C# and other languages
  • Possibility for streamed results / updates as tests finish even within a project
  • Could move test discovery and execution down to FSAC, allowing use across different clients

farlee2121 avatar Apr 11 '25 22:04 farlee2121

VSTest can run both VSTest-based projects and any projects with dual MTP/VSTest support, which seems like everything but TUnit.

Well, it's both TUnit, and can be xunit when xunit.runner.visualstudio isn't there.

The fact that MSTest and NUnit supports both is just for historical reasons. I can speak of MSTest, I'm looking forward to separate the support of MTP and VSTest.

If a project supports MTP, it's expected for Test Explorer to use MTP to run. Note that it's just not an implementation detail for which do you use to run tests, it can have real behavior differences.

MTP didn't seem like it could run VSTest-based projects.

The two platforms are completely different. None of them supports or can run the other. The fact that MSTest, NUnit, and in some cases xUnit, can run with both is just a choice that framework authors did.

But it's your product, you can choose what you want to support and what not to support.

Youssef1313 avatar Apr 12 '25 05:04 Youssef1313

  • We can remove all our code analysis for locating tests, instead relying on the code location built into each library's test adapter

For what it's worth, I think that would currently give much worse results for Expecto tests (go to location currently works quite well in Ionide, but is quite limited in Visual Studio with the YoloDev adaptor - e.g. https://github.com/YoloDev/YoloDev.Expecto.TestSdk/issues/107 / https://github.com/haf/expecto/issues/487) - I don't know how much effort it would take to get Expectos build in location reporting to work in all these cases

Numpsy avatar May 05 '25 21:05 Numpsy

Fair. We could leave the AST-based analysis as a test data source in Ionide until we improve code location through the test adapter, which should then reflect any improvements across all the major testing experiences (i.e. VS and Rider)

I know that YoloDev's code location all comes back to the getLocation method in Expecto.Impl, but I don't know it well enough to outline what improvements would look like.

farlee2121 avatar May 06 '25 22:05 farlee2121

I tried debugging through it a while back , and tbh i wasn't sure if all the issues are in Expecto itself or if there were some issues with the debug sequence points in the pdb files (Visual Studio sometimes doesn't like setting break points on all the tests which I think is a symptom of that, and I do see some bugs open in the compiler in that area, but this is just a guess, I don't know much about how that all works)

fwiw I wondered if it would be possible to do anything with CallerFilePath etc in Expecto given that the tests are functions, but it didn't seem like it would work without changing the API to make the otional parameters work

Numpsy avatar May 15 '25 15:05 Numpsy

Thanks for the insight!

That's an interesting idea. Optional parameters could get messy because we'd have to switch to static classes and tupled arguments.

I wonder if __SOURCE_FILE__ and related values would work if we could make the test methods inline?

It seems like that could be fragile or would have unintuitive consequences (all test function wrappers would need to be inline, existing reflection would be broken, what happens with the builders?)

farlee2121 avatar May 15 '25 18:05 farlee2121

Many of the existing functions (testCase and such) are already inline, but I thought those SOURCE type things used the original source, not where it's inlined into? (would have to test to make sure)

The existing C# wrapper functions might actually have an easier time with those sort of compile time changes compared to the base F# versions, as they're already using member functions with tupled arguments!

Numpsy avatar May 18 '25 10:05 Numpsy