-
Notifications
You must be signed in to change notification settings - Fork 42
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
Convert to .NET Core 3.0 #86
base: master
Are you sure you want to change the base?
Conversation
@jaredpar @jmarolf @DustinCampbell Should convert the other projects of course, and no idea if the nuget package generation still works (probably not). |
<Project>{a1f579f4-443e-4f64-bc55-998ab86ff293}</Project> | ||
<Name>xunit.runner.data</Name> | ||
</ProjectReference> | ||
<PackageReference Include="xunit.runner.utility" Version="2.4.1" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bradwilson it looks like AssemblyHelper
went internal between 2.1.0 and 2.4.1. Any idea on what we're supposed to use instead?
Given dependency resolution order (especially in .NET Core), it wasn't possible for that class to exist in xunit.runner.utility.dll, because it is required to successfully locate and load xunit.runner.utility.dll. So it became an internal class that was imported into each runner. There is really no way for us to provide this as a DLL to you as a result. :( |
So, what's a third party runner to do? Currently we use it here: https://github.com/Pilchie/xunit.runner.wpf/blob/master/xunit.runner.worker/XunitUtil.cs#L12 |
@jaredpar seems like once this is merged, you could do your original plan to multi-target the worker process and have it run on either .NET Core or .NET Framework, or maybe both (though that might be difficult, as you'd need different publish directories?) |
<TargetFrameworkProfile /> | ||
<NuGetPackageImportStamp> | ||
</NuGetPackageImportStamp> | ||
<TargetFramework>net452</TargetFramework> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any particular reason you're keeping this at such an old version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It had to do with one of the NuGet package dependencies. I think it was the XUnit issue that I'm discussing with @bradwilson.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, updated to net472 for the worker and netstandard2.0 for data and the sample unit test assembly.
We can't provide you with a binary that works, so you'll have to take a copy of the source for yourself, I'm afraid. 😞 |
Yeah I should get on the multi-target idea so that we can support CoreCLR here. |
Need to figure out how to get appveyor to install and use a local Note that it stops making it a package that you can reference, and turns it into a global tool instead. |
@bradwilson I did that for desktop - I took a peek at doing it for .NET Core, but it seems like that pulled in a bunch of other dependencies (in the form of |
...and that .NET Core code is the code you can't really trust to always do the right thing, I'm afraid. It's why we killed |
No description provided.