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

Use Xunit runner that is compatible with .NET 5 #5908

Merged
merged 2 commits into from Aug 24, 2020
Merged

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Aug 6, 2020

Using < 2.4.3 version of the runner fails with "Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again." when executing dotnet test on .NET 5 project. This version was specifically released to address this. See xunit/visualstudio.xunit#229.

Using the 2.4.1 version of the runner fails with "Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again." when executing `dotnet test` on .NET 5 project. 2,4,3 was specifically released to address this. See xunit/visualstudio.xunit#229.
@@ -86,7 +86,7 @@
<MicrosoftTestPlatformVersion Condition="'$(MicrosoftTestPlatformVersion)' == ''">16.5.0</MicrosoftTestPlatformVersion>
<XUnitVersion Condition="'$(XUnitVersion)' == ''">2.4.1</XUnitVersion>
<XUnitRunnerConsoleVersion Condition="'$(XUnitRunnerConsoleVersion)' == ''">$(XUnitVersion)</XUnitRunnerConsoleVersion>
<XUnitRunnerVisualStudioVersion Condition="'$(XUnitRunnerVisualStudioVersion)' == ''">$(XUnitVersion)</XUnitRunnerVisualStudioVersion>
<XUnitRunnerVisualStudioVersion Condition="'$(XUnitRunnerVisualStudioVersion)' == ''">2.4.3</XUnitRunnerVisualStudioVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be required for testing on 5.0, but needs a comment explaining why it's not the being set to $(XUnitVersion)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just.....update XUnitVersion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@MattGal MattGal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a comment but I think we should get @tmat to approve before merging.

@pranavkm
Copy link
Contributor Author

pranavkm commented Aug 7, 2020

@tmat can you have a look? We're working around this in aspnetcore, but the experience is really problematic. dotnet test runs fail silently

@lpatalas
Copy link
Contributor

@pranavkm @tmat Is there something blocking this PR from being merged? It would be great to have this change applied in Arcade.

@pranavkm
Copy link
Contributor Author

@lpatalas I'd be happy for some one to merge this change. We've been using this version of Xunit for quite some time in dotnet/aspnetcore without issues.

@MattGal
Copy link
Member

MattGal commented Aug 24, 2020

@lpatalas I'd be happy for some one to merge this change. We've been using this version of Xunit for quite some time in dotnet/aspnetcore without issues.

I went ahead and approved.

@pranavkm pranavkm merged commit d8f74bf into master Aug 24, 2020
@mmitche mmitche deleted the pranavkm-patch-1 branch December 17, 2020 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants