-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Update SkipOnCoreClrAttributes to new parameters #28875
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,5 +4,5 @@ | |
|
|
||
| using Xunit; | ||
|
|
||
| [assembly: SkipOnCoreClr("System.Net.Tests are flaky and/or long running: https://github.com/dotnet/runtime/issues/131")] | ||
| [assembly: SkipOnCoreClr("System.Net.Tests are flaky and/or long running: https://github.com/dotnet/runtime/issues/131", RuntimeTestModes.CheckedRuntime)] | ||
| [assembly: SkipOnMono("System.Net.Tests are flaky and/or long running: https://github.com/dotnet/runtime/issues/131")] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we know if mono actually had problems with these or just copied the coreclr attribution thinking it was about all configs and not just stress?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My assumption here is that mono did have problems with this, and
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I just copied from CoreCLR so this might not be needed. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it just checked builds, or any of the stress / non-release configurations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would apply to ALL Checked builds, no matter if it is stress or not.
However, I just realized that the current design is not great as if we added a
| RuntimeTestModes.GCStressit would mean, ALL Checked or GCStress (Checked or Release), as I would think what we want is that if I sayRuntimeTestModes.Checkedit would mean, ALL Checked, but if I add an stress mode flag, then it would mean, ONLY skip when runningGCStresswhen inChecked. What is your expectation/thought for that?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stephentoub thoughts on that?
I'll include this change in the arcade update to unblock and can update again based on your thoughts. I think updating it to the latter is the ideal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I gave it a thought and put up a PR to update it to a better way of doing it: dotnet/arcade#4764