-
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
Skipping Json StreamTests.HandleCollectionsAsync test #35927
Comments
Tagging subscribers to this area: @jozkee |
Will be disabled in #35568. |
Hanging again this time on CoreCLR: https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-43185-merge-1b23d2c75eb24a8891/System.Text.Json.Tests/console.cb4a64bf.log?sv=2019-07-07&se=2020-10-30T02%3A28%3A54Z&sr=c&sp=rl&sig=aoJRcu35I697idbF6olx02wDyhDhgUZE7gRU85kvQlQ%3D
|
I created an issue for the CoreCLR checked hangs as it is happening pretty often, it needs attention and there are more tests than just |
The test starting hanging in net461 debug test runs, curiously after dfd618d got merged. We might consider either refactoring the test or disabling it for net461 as well. cc @layomia @stephentoub @ericstj |
It's because System.Text.Json apparently reaches into Corelib's directory to include: When compiled for .NET 6, where we have the new interpolated string handler for Debug.Assert, this:
will only ever be executed when an assert fails. But pre-.NET 6, the Debug.Assert message strings that access that ToHexString method will invoke it for every assert, whether it passes or fails. |
@eiriktsarpalis can this be closed? It appears UnicodeDebug.cs was reverted in #55954 |
#55954 addressed a performance regression that made net461 runs time out, however this particular test is known to time out in other platforms even before that regression got introduced. We should keep this open for now, @steveisok is it ok if we remove this from 6.0.0 milestone? |
Yes, that is fine to remove the 6 milestone. |
This test fails as a long running test and will be skipped. This issue exists to make sure we address it moving forward.
The text was updated successfully, but these errors were encountered: