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

Test infrastructure #88

Merged
merged 20 commits into from
Sep 19, 2023
Merged

Test infrastructure #88

merged 20 commits into from
Sep 19, 2023

Conversation

dawedawe
Copy link
Contributor

@dawedawe dawedawe commented Sep 11, 2023

This refactors some code to make unit testing with the SDK easier.
To make really sure we are using valid FSharpProjectOptions with all platforms and framework versions we make some effort to create a temporary project and read the options from it.
This is currently not working for net8, see here

- add a sample unit test project for the option sample
# Conflicts:
#	src/FSharp.Analyzers.Cli/Program.fs
#	src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.Client.fs
#	src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fs
@dawedawe dawedawe marked this pull request as ready for review September 15, 2023 12:14
Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

Some nits left but I'm really happy with this setup.
Please consider adding some documentation about this as well.
Something a la "Write your first unit test"

- rename module "AssertionHelpers" to "Assert"
- rename funcition "areWarningsInLines" to "hasWarningsInLines"
- shuffle order of "hasWarningsInLines"
Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

Great work! I'm going to merge this in to play around with the testing story in my current work.

@nojaf nojaf merged commit 94adc92 into ionide:master Sep 19, 2023
2 checks passed
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.

4 participants