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

Add skipping tests based on the [SupportedOSPlatform] attribute #48

Merged
merged 4 commits into from
Nov 30, 2024

Conversation

0xced
Copy link
Contributor

@0xced 0xced commented Nov 28, 2024

Adding support for SupportedOSPlatform is great because this attributes suppresses the CA1416 code analysis warning.

This feature was requested in xunit/xunit#2820 but was not implemented in xUnit.net

Important

I documented in the README that this feature is available since version 1.5. Please update the README to match the actual version number of the next release.

Ping @sliekens who might be interested in this feature. 😉

@0xced 0xced force-pushed the SupportedOSPlatform branch from 0af73c9 to ddf4349 Compare November 28, 2024 20:13
Adding support for `SupportedOSPlatform` is a great because this attributes suppresses the CA1416 code analysis warning.

This feature was requested in xunit/xunit#2820 but was not implemented in xUnit.net
@0xced 0xced force-pushed the SupportedOSPlatform branch from ddf4349 to 34c92ae Compare November 28, 2024 20:18
@0xced 0xced force-pushed the SupportedOSPlatform branch from e9ad3f8 to 2295987 Compare November 29, 2024 10:46
Copy link
Owner

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Looks amazing! Thank you.

@AArnott AArnott enabled auto-merge November 30, 2024 15:21
@AArnott AArnott disabled auto-merge November 30, 2024 15:35
@AArnott AArnott merged commit 2965bfa into AArnott:main Nov 30, 2024
4 checks passed
@0xced 0xced deleted the SupportedOSPlatform branch November 30, 2024 16:08
@0xced
Copy link
Contributor Author

0xced commented Nov 30, 2024

Awesome, thanks for merging! Do you plan to release a new (1.5) version on NuGet anytime soon?

@AArnott
Copy link
Owner

AArnott commented Nov 30, 2024

Yes, with your addition I think I'll push the release today.

@0xced
Copy link
Contributor Author

0xced commented Nov 30, 2024

I see that version 1.5.22-g0ae9b5ba5c was released. Did you actually mean to publish a prerelease? I read a bit about Nerdbank.GitVersioning, especially Public vs. stable releases and version.json file but I must admit that as a MinVer user I'm a bit lost. 🤨

@AArnott
Copy link
Owner

AArnott commented Nov 30, 2024

Ah phooey. I meant it to be stable. I changed my release process and apparently there's a bug. I'll get that fixed and re-release.

@sliekens
Copy link

I also highly recommend MinVer, though I never tried Nerdbank.GitVersioning, it's very hard to make a mistake with MinVer.

@0xced
Copy link
Contributor Author

0xced commented Nov 30, 2024

@sliekens Something tells me that Andrew is going to stick with Nerdbank.GitVersioning. 😉

And thanks @AArnott for releasing version 1.5.23! 🥳

@sliekens
Copy link

Oh well. I said what I said. All I can do now is double down on it.

@0xced
Copy link
Contributor Author

0xced commented Nov 30, 2024

And I just opened the first pull request ever to use this new feature! 🤓 MarkusPalcer/blurhash.net#28

@AArnott
Copy link
Owner

AArnott commented Dec 1, 2024

Oh! I hadn't noticed that it works when the SupportedOSPlatform attribute is added to the test class as well. Very cool.
We should call that out on the doc page.

@0xced
Copy link
Contributor Author

0xced commented Dec 1, 2024

I documented this behaviour in the private TestMethodExtensions.GetPlatforms method which is admittedly not the most discoverable place. 😆

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

Successfully merging this pull request may close these issues.

3 participants