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

Support Microsoft.Testing.Platform for xunit v2 #416

Open
MarcoRossignoli opened this issue Sep 10, 2024 · 10 comments
Open

Support Microsoft.Testing.Platform for xunit v2 #416

MarcoRossignoli opened this issue Sep 10, 2024 · 10 comments
Labels

Comments

@MarcoRossignoli
Copy link

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 #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

@bradwilson
Copy link
Member

bradwilson commented Sep 11, 2024

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
Copy link
Member

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

@MarcoRossignoli
Copy link
Author

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?

@bradwilson
Copy link
Member

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

@Youssef1313
Copy link

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 microsoft/vs-extension-testing#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?

@bradwilson
Copy link
Member

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. 😄

@Youssef1313
Copy link

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.

@bradwilson
Copy link
Member

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.

@Youssef1313
Copy link

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?

@bradwilson
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants