Fix unsupported type detection when serializing polymorphic System.Type instances #67618
Conversation
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsFix #67588.
|
| @@ -72,6 +72,7 @@ async Task RunTest<T>(string json) | |||
| [Fact] | |||
| public async Task SerializeUnsupportedType() | |||
| { | |||
| // TODO refactor to Xunit theory | |||
There was a problem hiding this comment.
could this be done as part of this PR?
There was a problem hiding this comment.
It's a pervasive pattern so I'd rather it be done in a separate PR.
| @@ -124,6 +124,20 @@ async Task RunTest<T>(T value) | |||
| serialized = await Serializer.SerializeWrapper(obj, new JsonSerializerOptions { IgnoreNullValues = true }); | |||
| Assert.Equal(@"{}", serialized); | |||
| } | |||
|
|
|||
| #if !BUILDING_SOURCE_GENERATOR_TESTS | |||
There was a problem hiding this comment.
Why isn't this being run in source-gen mode?
There was a problem hiding this comment.
Polymorphic scenaria will throw InvalidOperationException before we get the chance to exercise the changed codepath.
There was a problem hiding this comment.
Got it. FWIW when there are consequential divergences in source-gen/reflection behavior, we add a separate test variant for source-gen, e.g. here.
There was a problem hiding this comment.
Got it. I'm familiar with that pattern but FWIW it seems to encourage repetition of test definitions (or even divergence because only one implementation gets updated like in this case). Going forward I'd prefer it if we could avoid overriding test methods and either use compile conditionals or runtime parameterization (e.g. if (this.SerializerWrapper.IsSourceGenerated) ...)
|
Test failures unrelated to Json |
Fix #67588.