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

System.Text.Json.SourceGeneration.Tests have issues with BigInteger and records on netfx #63802

Closed
Tracked by #77019
krwq opened this issue Jan 14, 2022 · 3 comments · Fixed by #87640
Closed
Tracked by #77019
Labels
area-System.Text.Json source-generator Indicates an issue with a source generator feature
Milestone

Comments

@krwq
Copy link
Member

krwq commented Jan 14, 2022

Following issues were found when enabling netfx tests:

Whenever:

[JsonSerializable(typeof(ClassWithUnsupportedBigInteger))]
// and couple of other BigInteger related classes 

is used the code no longer compiles - code generator seem to be generating code which tries to access private fields. Search for #if !NETFRAMEWORK in that code - this have been removed for netfx for now to unblock remainder of the tests

also JsonIgnoreAttribute_UnsupportedBigInteger is not passing - expected exception seem to differ. See test with ActiveIssue.

Another issues I'm seeing are InvalidOperationException (emit fails in CreateAssemblyImage in the test code), following tests are affected:

  • System.Text.Json.SourceGeneration.UnitTests.GeneratorTests.RecordInExternalAssembly
  • System.Text.Json.SourceGeneration.UnitTests.GeneratorTests.Record
  • System.Text.Json.SourceGeneration.Tests.JsonSerializerContextTests.Converters_AndTypeInfoCreator_NotRooted_WhenMetadataNotPresent (I'm not able to reproduce this one locally)

(those tests were recently added)

Related to: #63463

cc: @layomia

Note: this issue is related to the PR which is not merged yet so some details reated to ActiveIssue or DefineConstants are relevant after merging it. (I need ActiveIssue number in order to finish PR)

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 14, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Jan 14, 2022

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

Issue Details

Following issues were found when enabling netfx tests:

Whenever:

[JsonSerializable(typeof(ClassWithUnsupportedBigInteger))]
// and couple of other BigInteger related classes 

is used the code no longer compiles - code generator seem to be generating code which tries to access private fields. Search for BUILDING_NETFX in that code - this have been removed for netfx for now to unblock remainder of the tests

also JsonIgnoreAttribute_UnsupportedBigInteger is not passing - expected exception seem to differ. See test with ActiveIssue.

Related to: #63463

cc: @layomia

Note: this issue is related to the PR which is not merged yet so some details reated to ActiveIssue or DefineConstants are relevant after merging it. (I need ActiveIssue number in order to finish PR)

Author: krwq
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@krwq krwq changed the title System.Text.Json.SourceGeneration.Tests have issues with BigInteger on netfx System.Text.Json.SourceGeneration.Tests have issues with BigInteger and records on netfx Jan 17, 2022
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Jan 20, 2022
@layomia layomia added this to the 7.0.0 milestone Jan 20, 2022
@layomia layomia self-assigned this Jan 20, 2022
@layomia layomia added the source-generator Indicates an issue with a source generator feature label Jan 21, 2022
@eiriktsarpalis eiriktsarpalis modified the milestones: 7.0.0, Future Apr 18, 2022
@layomia layomia removed their assignment Dec 2, 2022
@eiriktsarpalis
Copy link
Member

The BigInteger failures were fixed by #87383 and the record unit tests are caused by the fact that netfx doesn't have the IsExternalInit class defined. I'll follow up with a PR enabling the tests and closing the issue.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 15, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 15, 2023
@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 8.0.0 Jun 22, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants