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

Update Xunit version, migrate FSharpSuite.Tests to Xunit #17652

Merged
merged 11 commits into from
Sep 3, 2024

Conversation

majocha
Copy link
Contributor

@majocha majocha commented Sep 2, 2024

Updates Xunit to 2.9.0.

Migrate FSharpSuite.Tests to Xunit (But the parallelization is still disabled).

In line with:
#13654
#9947

Copy link
Contributor

github-actions bot commented Sep 2, 2024

✅ No release notes required

@majocha majocha changed the title bump xunit version, move suite to xunit Update Xunit version, convert FSharpSuite.Tests to Xunit Sep 2, 2024
@majocha majocha changed the title Update Xunit version, convert FSharpSuite.Tests to Xunit Update Xunit version, migrate FSharpSuite.Tests to Xunit Sep 2, 2024
@majocha
Copy link
Contributor Author

majocha commented Sep 2, 2024

@psfinaki this surprisingly worked.

I'm not sure, is it ok to upgrade the Xunit version? The one we are on currently is seriously old.

Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

This is brilliant, Jakub, brilliant, thanks for doing this! 🚀 🚀 🚀

Regarding xunit update, that's also definitely a good thing :)

tests/fsharp/NUnitHelpers.fs Outdated Show resolved Hide resolved
tests/fsharp/NUnitHelpers.fs Outdated Show resolved Hide resolved
tests/fsharp/tests.fs Outdated Show resolved Hide resolved
tests/fsharp/tests.fs Outdated Show resolved Hide resolved
tests/fsharp/tests.fs Show resolved Hide resolved
@psfinaki
Copy link
Member

psfinaki commented Sep 2, 2024

Wait @majocha, just for my understanding, did you jump on this because I was recently crying for help on this topic at this meetup? :D

@majocha
Copy link
Contributor Author

majocha commented Sep 2, 2024

@psfinaki I might have been somewhat influenced by the video xD.

@psfinaki
Copy link
Member

psfinaki commented Sep 2, 2024

Ohhh... There I promised to visit that person in their city and buy them a beer 🤔 Okay I will reach out to you offline once we merge this :D

@majocha majocha marked this pull request as ready for review September 3, 2024 08:41
@majocha majocha requested a review from a team as a code owner September 3, 2024 08:41
Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

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

Awesome job :)

It's really a surprise this worked out without bigger hiccups, such a surprise that I manually checked that tests execute and pass in the CI but yeah they do :D

@psfinaki psfinaki merged commit 79c64d9 into dotnet:main Sep 3, 2024
30 checks passed
@vzarytovskii
Copy link
Member

Hm, so

This PR: 72,238
Previous run in main: 72,998

@majocha from the top of your head, do you know why it's around 600 tests less reported?

@T-Gro
Copy link
Member

T-Gro commented Sep 3, 2024

https://dev.azure.com/dnceng-public/public/_build/results?buildId=796072&view=logs&j=916a2273-64f0-5130-a29e-a4d2f7e48c60&t=b833feec-4728-51e1-390e-168ead0990a4&l=119538

  "D:\a\_work\1\s\tests\fsharp\FSharpSuite.Tests.fsproj" (VSTest target) (1) ->
   (_VSTestMSBuild target) ->
     D:\a\_work\1\s\.dotnet\sdk\9.0.100-preview.7.24407.12\Microsoft.TestPlatform.targets(46,5): warning : [xUnit.net 00:00:00.72] Skipping: FSharpSuite.Tests (could not load dependent assembly 'FSharpSuite.Tests, Version=12.9.100'): Could not load file or assembly 'FSharpSuite.Tests, Version=12.9.100.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. Strong name signature could not be verified.  The assembly may have been tampered with, or it was delay signed but not fully signed with the correct private key. (Exception from HRESULT: 0x80131045) [D:\a\_work\1\s\tests\fsharp\FSharpSuite.Tests.fsproj::TargetFramework=net472]
     D:\a\_work\1\s\.dotnet\sdk\9.0.100-preview.7.24407.12\Microsoft.TestPlatform.targets(46,5): warning : No test is available in D:\a\_work\1\s\artifacts\bin\FSharpSuite.Tests\Release\net472\FSharpSuite.Tests.dll. Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again. [D:\a\_work\1\s\tests\fsharp\FSharpSuite.Tests.fsproj::TargetFramework=net472]

@majocha
Copy link
Contributor Author

majocha commented Sep 3, 2024

@vzarytovskii by the number it is about one entire target of FSharpSuite not running.

@psfinaki
Copy link
Member

psfinaki commented Sep 3, 2024

This might be because the xunit runner is not configured, I am checking this locally

@majocha
Copy link
Contributor Author

majocha commented Sep 3, 2024

Yes, there is no xunit.runner.json.

@psfinaki
Copy link
Member

psfinaki commented Sep 3, 2024

Trying this out here: #17658

@majocha
Copy link
Contributor Author

majocha commented Sep 3, 2024

possibly something like this dotnet/arcade#14091

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

Successfully merging this pull request may close these issues.

4 participants