-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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 back fixed xunit decimal, IntPtr and UIntPtr commented test cases #87084
Conversation
Could you look at the build errors? |
@danmoseley Sorry! Looking at those now, putting this as a draft PR until I get that fixed. |
@danmoseley The failing tests are related to Native AOT deployments. I'm not familiar with this kind of deployment, but it seems odd that it only fails with the Decimal type. Are these tests failing expected behavior or am I missing some Native AOT deployment specific configuration? |
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.
LGTM when CI is happy
|
I pushed a change that will likely fix it, but I didn't attempt to test. we'll see. |
Tagging subscribers to this area: @dotnet/area-meta Issue DetailsCloses #27187 These tests were removed due to an issue in xunit that is now resolved on the latest version. The test projects are also on the latest xunit version, so all that is needed is to uncomment the marked tests.
|
@joaonunomota I should be able to merge tomorrow. Thanks for your first contribution! Are you interested in another? We have quite a few issues marked "help wanted" and also a fair number marked "api-approved" that would be adding a new API to .NET. You are welcome to grab any not already assigned. |
@danmoseley Thank you for all the help! I'll have a look through those other issues |
@dotnet/dnceng a variety of flavors ran over 4 hours here and timed out. this is the 2nd time around. is there a known issue -- does this mean low capacity, or slow machines? |
Yes: dotnet/arcade#13774 |
Unrelated
|
@stephentoub Any thoughts about above stack? |
Seems like there's a race condition that may have happened during the assert; the assert is checking for null and then checking for something else, and it looks most likely that the value changed to null just after it was checked for null, and thus failed. The assert is easily fixable. I'm more concerned about in what situation there could be a race condition here... it's not immediately obvious. |
Thanks. I'll not open an issue since it's not actionable -- with your assert, it will be next time. |
Closes #27187
These tests were removed due to an issue in xunit that is now resolved on the latest version. The test projects are also on the latest xunit version, so all that is needed is to uncomment the marked tests.