visualstudio.xunit icon indicating copy to clipboard operation
visualstudio.xunit copied to clipboard

Support `Microsoft.Testing.Platform` for xunit v2

Open MarcoRossignoli opened this issue 1 year ago • 10 comments

Hi @bradwilson,

we could have some team that are interested to onboard the new testing platform but they're stuck at the moment with xunit v2.

We did integration with v3 already, I wonder if it could be fine to you if we take the integration of the v2 using the bridge(that won't add more dependencies than what you saw in v2) to have same/similar experience also for users that cannot at the moment move to v3. Let's say have v2 onboarded like MSTest and nUnit and v3 that's anyway a breaking change with the new experience.

The idea is to do something like https://github.com/xunit/visualstudio.xunit/pull/403 under your "supervision", we create the PRs and you do the review, so you don't have to spend coding time on it.

cc: @pavelhorak

MarcoRossignoli avatar Sep 10 '24 10:09 MarcoRossignoli

If this can be done without breaking xUnit.net v3 ~(especially the microsoft-testing-platform branch)~, then I would be willing to entertain a PR to do this.

Edit: The microsoft-testing-platform branch has been merged into main, and is now part of build 0.4.0-pre.10 and later.

bradwilson avatar Sep 11 '24 02:09 bradwilson

I would also be interested to know what the adoption blockers are for users who cannot use v3.

bradwilson avatar Sep 11 '24 02:09 bradwilson

I would also be interested to know what the adoption blockers are for users who cannot use v3.

Do you have documentation on the breaking changes?

MarcoRossignoli avatar Sep 11 '24 16:09 MarcoRossignoli

Most of the breaking changes should be here: https://xunit.net/docs/getting-started/v3/migration

And there's also a What's New page that can be helpful for people moving: https://xunit.net/docs/getting-started/v3/whats-new

bradwilson avatar Sep 11 '24 20:09 bradwilson

I would also be interested to know what the adoption blockers are for users who cannot use v3.

There is some extensive usage of extensibility APIs in some repos. I experimented with migrating vs-extension-testing (see https://github.com/microsoft/vs-extension-testing/pull/179). As you see, the PR is MASSIVE, and isn't yet even near to completion (Am I'm missing something obvious for the changes I'm making?).

Back on topic though, are you generally okay with adding MTP support for xunit v2?

Youssef1313 avatar Mar 18 '25 11:03 Youssef1313

As you see, the PR is MASSIVE

Obviously it's up to you to decide how to achieve your technical goals, but trying to mix v2 and v3 support into a single library is probably the wrong strategy. A quick glance through the PR shows a ton of conditional code. I don't know anything about this library or how it's used, so I may be speaking from ignorance here, but going down this path is likely a significant source of your pain. 🤷‍♂

Back on topic though, are you generally okay with adding MTP support for xunit v2?

I remain skeptical that modifying xunit.runner.visualstudio to add support for MTP for v2 is even technically possible, but y'all are the ones who want to try to do it. 😄

bradwilson avatar Mar 19 '25 00:03 bradwilson

trying to mix v2 and v3 support into a single library is probably the wrong strategy.

For now, I was keeping the old packages to support v2, and introducing a new package to support v3. Yes conditional code is kinda annoying, but still the number of needed changes is huge.


I remain skeptical that modifying xunit.runner.visualstudio to add support for MTP for v2 is even technically possible, but y'all are the ones who want to try to do it. 😄

Can I have better understanding of the concerns? Assuming I managed to get it working, so it was "technically possible", are there other concerns that could prevent you from merging and shipping that work for v2? Is there another approach you prefer? (e.g, introducing a completely new package xunit.v2.mtp?, or plugging the support via another existing package?)

I haven't really looked yet into how xunit is structured, but from a high-level point of view, the xunit.runner.visualstudio seems a good candidate as it can leverage the support for VSTestBridge, which will make the work significantly easier.

Youssef1313 avatar Mar 19 '25 05:03 Youssef1313

Can I have better understanding of the concerns?

If you're planning to use the VSTest adapter, then it will insist on making changes that are not compatible to v3, and xunit.runner.visualstudio supports all of v1, v2, and v3.

Assuming I managed to get it working, so it was "technically possible", are there other concerns that could prevent you from merging and shipping that work for v2?

I would be very happy to look at a PR that makes it work, and if it's something I'm comfortable supporting, then also ship it.

Is there another approach you prefer?

I'm not interested in sweeping changes to v2, as I have no plans for any further support of v2. I suspect your suggestion of xunit.v2.mtp would be sweeping changes to the core of v2, which I definitely am not interested in accepting or supporting. From my perspective, the answer to "how do I get MTP support?" is "use v3". If there's a quick and non-intrusive way that you can add it for v2, then I'm happy to look at it, but not at the expensive of massive changes and shipping new versions of v2 itself.

bradwilson avatar Mar 19 '25 19:03 bradwilson

then it will insist on making changes that are not compatible to v3

To make sure I'm following, are you referring to runsettings and VSTest filter?

Youssef1313 avatar Mar 19 '25 19:03 Youssef1313

To make sure I'm following, are you referring to runsettings and VSTest filter?

No, we support those today.

Last I looked, the VSTest => MTP adapter required injection of the MTP entry point and conversion to an executable. v3 projects already natively support MTP (without needing xunit.runner.visualstudio, which is considered to be purely a VSTest adapter), and I don't believe there's any way to inject that code only into v2 projects without mandating a separate package for only v2. If that's the strategy that's necessary to support MTP for v2, then Microsoft can own, support, and publish that package on their own.

bradwilson avatar Mar 20 '25 01:03 bradwilson