Add SkipOnCoreclr and SkipOnMono attributes#4231
Merged
safern merged 7 commits intodotnet:masterfrom Nov 18, 2019
Merged
Conversation
jkotas
reviewed
Oct 30, 2019
src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCoreClrDiscoverer.cs
Outdated
Show resolved
Hide resolved
jkotas
reviewed
Oct 30, 2019
src/Microsoft.DotNet.XUnitExtensions/src/RuntimeStressTestModes.cs
Outdated
Show resolved
Hide resolved
Member
Author
|
cc: @marek-safar |
danmoseley
reviewed
Oct 30, 2019
danmoseley
reviewed
Oct 30, 2019
src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCoreClrDiscoverer.cs
Outdated
Show resolved
Hide resolved
danmoseley
reviewed
Oct 30, 2019
src/Microsoft.DotNet.XUnitExtensions/src/Attributes/SkipOnMonoAttribute.cs
Outdated
Show resolved
Hide resolved
marek-safar
reviewed
Oct 31, 2019
src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCoreClrDiscoverer.cs
Outdated
Show resolved
Hide resolved
marek-safar
reviewed
Oct 31, 2019
src/Microsoft.DotNet.XUnitExtensions/src/RuntimeStressTestModes.cs
Outdated
Show resolved
Hide resolved
ViktorHofer
reviewed
Oct 31, 2019
src/Microsoft.DotNet.XUnitExtensions/src/Attributes/SkipOnCoreClrAttribute.cs
Show resolved
Hide resolved
ViktorHofer
reviewed
Oct 31, 2019
src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCoreClrDiscoverer.cs
Outdated
Show resolved
Hide resolved
BruceForstall
suggested changes
Nov 2, 2019
src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCoreClrDiscoverer.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCoreClrDiscoverer.cs
Outdated
Show resolved
Hide resolved
ViktorHofer
approved these changes
Nov 4, 2019
src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCoreClrDiscoverer.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCoreClrDiscoverer.cs
Outdated
Show resolved
Hide resolved
Member
|
What's the remaining work here? |
Member
Author
I'm finishing local testing (found a small couple bugs), will update and merge. |
Member
Author
|
@BruceForstall @jkotas @stephentoub @marek-safar can I get a review just to make sure this change meets all we need the way it is and see if you can spot any wrong assumptions? |
stephentoub
approved these changes
Nov 13, 2019
src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCoreClrDiscoverer.cs
Show resolved
Hide resolved
|
|
||
| return assemblyConfigurationAttribute != null && | ||
| string.Equals(assemblyConfigurationAttribute.Configuration, "Checked", StringComparison.InvariantCulture); | ||
| } |
Member
There was a problem hiding this comment.
Should we cache the answer to this and some of the other IsXx properties/methods? Seems like they could end up being invoked many times while running tests and potentially affect test execution time. It could always be done later, though, if it's found to be an issue.
Member
Author
There was a problem hiding this comment.
Yeah, I was thinking about that. I will explore on doing it before merging the PR.
Member
Author
|
Merging as it is to unblock ongoing work. Will add caching later this week. I'll open an issue for that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This contributes toward unifying the way we disable tests in between CoreFX/CoreCLR and Mono testing for corefx tests.
The way it works is as follows:
SkipOnCoreClrAttribute
We can use this attribute in different ways:
SkipOnMonoAttribute
We can use this attribute in different ways:
The way these attributes work, if the attribute is added and the conditions are met, we will add a trait to the member which it was added (assembly, class or method)
Category=failing. Then when calling XUnit as we already do today, we need to pass an argument,-notrait Category=failing.I'm still testing this locally but wanted to start gathering feedback on the proposal.
cc: @jkotas @BruceForstall @stephentoub @jaredpar @ViktorHofer @danmosemsft