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

[STJ SourceGen] Use Roslyn4.4 target when testing #79890

Closed

Conversation

eiriktsarpalis
Copy link
Member

@eiriktsarpalis eiriktsarpalis commented Dec 21, 2022

Created this PR to try to isolate the test failures reported in #79828 (comment)

@ghost
Copy link

ghost commented Dec 21, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json

Milestone: -

Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

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

I'm OK with not testing the roslyn4.0 source generator for now. We do the same for target frameworks and only test the very latest. If we notice regressions in the roslyn4.0 source generator though (i.e. based on customer reports), we should invest in testing it as well.

@eiriktsarpalis
Copy link
Member Author

I neglected to put in the description that I created this PR in order to isolate some wasm test failures I've been seeing: #79828 (comment)

TL;DR running tests against Roslyn4.4 causes deterministic assertion failures when running release builds on wasm. I don't see how a compile time artifact such as the Roslyn version could trigger test failures at runtime, but I'll keep investigating.

@eiriktsarpalis eiriktsarpalis added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Dec 22, 2022
@build-analysis build-analysis bot mentioned this pull request Dec 23, 2022
@eiriktsarpalis
Copy link
Member Author

Hi @dotnet/mono-team, it looks like the changes introduced in 5236bb8 are causing a number of tests to fail on wasm only. I've been able to narrow down the failures to a single unit test in fe3a627. The failure happens deterministically and only occurs on wasm targets. It appears to occur because the JsonDocument._utf8Json span is reported as empty, even though my changes in the second commit make this case impossible -- I suspect this could be caused by some kind of memory corruption.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants