search

xunit / visualstudio.xunit

VSTest runner for xUnit.net (for Visual Studio Test Explorer and dotnet test)
https://xunit.net/
Other
144 stars 81 forks source link

3-rd party reporter is broken in 2.5.1 release (2.5.2-pre.3) #385

Closed Aleh-Yanushkevich closed 11 months ago

Aleh-Yanushkevich commented 11 months ago

As was mentioned in #384, issue still representing for 3-rd party reporter reportportal/agent-net-xunit/issues/27. Project with issue reproducing is placed here.

Installing xunit.runner.reporters into the project has not fixed the issue but testing continues.

image

Runtime: .Net 6.0/7.0

bradwilson commented 11 months ago

Yeah, I think I understand what's going on, and it's probably worse than I originally thought.

For 2.5.1 when I was merging in xunit.runner.utility (in an attempt to prevent us from colliding with third parties), it didn't really dawn on me what the consequence of the merge process changing the identity of xunit.runner.utility would mean: that any attempt to load a third party reporter library would result in no reporters, because the identity of their copy of xunit.runner.utility and our internal copy would not match, so we would not find the reporters inside the library (they would effectively be implementing a different identity of IRunnerReporter and friends).

What's also a problem is that the "last copy wins" for xunit.runner.utility problem that I was trying to solve can't really be effectively solved at all. So, in reality, third party runner reporters are just outright broken, because we have not made any attempt to ensure that xunit.runner.reporter does not break any binary contracts from version to version.

Since your library (ReportPortal.XUnit) is linked against a specific version of xunit.runner.utility, it's possible that any "silent upgrades" of this library would break binary compatibility. You don't ship xunit.runner.utility (either in the .nupkg nor as a dependency listed in the .nuspec), so you're (a) counting on the runner you're slotting into to ship it, and (b) doing so in such a way that you have no idea whether you're getting a binary that'll be compatible with you.

So I think the end result is that I'm going to have to undo the merge, and bring back shipping the runner utility library with xunit.runner.visualstudio. You'll still be rolling the dice on whether your reporter will work, based on the version of xunit.runner.utility you build against vs. the one that's in use by (and shipped by) whatever runner you happen to be slotted into.

In truth, the "3rd party reporters" thing has never been something we fully fleshed out, and the fact that anybody has been able to use it with any level of success is more than a little surprising. Once I roll the changed back out for the merge, you may go back to working...and you may not. I have thoughts on things you could do, but none of them are great so I'm not ready to suggest you do any of them at this point. I'm just going to roll back the merge and we can see what happens.

I'll let you know in this issue when a build is available that you can test against.

bradwilson commented 11 months ago

@Aleh-Yanushkevich Here it is working with 2.5.2-pre.7:

image

Please try this on your larger project and let me know if it's working okay for you.

Commit for this rollback: https://github.com/xunit/visualstudio.xunit/commit/2dd69895010eaff737e38e567f7f792996cd3ba4

Aleh-Yanushkevich commented 11 months ago

@bradwilson thank you for detailed explanation of this behavior. I've tested on another project and it works fine.