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

Compiler tests cleanup, part 1 #15690

Merged
merged 4 commits into from
Jul 31, 2023
Merged

Compiler tests cleanup, part 1 #15690

merged 4 commits into from
Jul 31, 2023

Conversation

psfinaki
Copy link
Member

This will be quite a journey.

The idea is to merge FSharp.Compiler.Service.Tests with FSharp.Compiler.UnitTests and move to xUnit altogether.

This will also remove some test duplication and save some CI time.

@psfinaki psfinaki requested a review from a team as a code owner July 26, 2023 16:15
@vzarytovskii vzarytovskii requested review from nojaf and auduchinok July 26, 2023 16:30
@vzarytovskii
Copy link
Member

@auduchinok @nojaf could you please take a look at it, if it works for you guys. I don't think that UnitTests are in the FCS solution, so it's pretty much removing tests from this solution.

@auduchinok
Copy link
Member

@auduchinok @nojaf could you please take a look at it, if it works for you guys. I don't think that UnitTests are in the FCS solution, so it's pretty much removing tests from this solution.

Removing the tests project from FCS solution would be really problematic, as we mostly work with this solution. Would it be possible to add UnitTests to that solution instead?

@psfinaki
Copy link
Member Author

Uff that would be even harder - because FSharp.Compiler.Service.Tests is NUnit whereas we are converging things to xUnit, hence to FSharp.Compiler.UnitTests which is xUnit already.

How about including both projects in the FCS solution until we eventually only have one?

@KevinRansom
Copy link
Member

@psfinaki , hey mate, at first I was wondering why you wasn't merging unit tests into compiler service, because xunit has fewer files. But I suppose the xunit translation may take a moment.

Anyway two thumbs for this, I like the goal.

Copy link
Member

@vzarytovskii vzarytovskii left a comment

Choose a reason for hiding this comment

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

Both suites should be in the service solution.

@psfinaki
Copy link
Member Author

psfinaki commented Jul 27, 2023

Both suites should be in the service solution.

Done

@psfinaki psfinaki enabled auto-merge (squash) July 27, 2023 13:13
@psfinaki psfinaki requested a review from vzarytovskii July 27, 2023 13:13
@T-Gro T-Gro dismissed vzarytovskii’s stale review July 31, 2023 09:08

Project was added to Fsharp.Compiler.Service.sln

@psfinaki psfinaki merged commit 248b1a4 into dotnet:main Jul 31, 2023
@psfinaki psfinaki deleted the tests-3 branch July 31, 2023 09:42
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.

5 participants