Add analyzer to flag non-serializable test case implementations (xUnit3006/xUnit3007)

[ana1250]

Add analyzer to flag non-serializable test case implementations (xUnit3006/xUnit3007)

Closes xunit/xunit#3535

Summary

Adds a new diagnostic analyzer TestCaseMustBeSerializable that flags ITestCase implementations which are not serializable. Test cases must be serializable to support test discovery and execution.

Also fixes a pre-existing typo: AnalayzeSerializability → AnalyzeSerializability.

New Diagnostics

Rule	Severity	Title	When
xUnit3006	Warning	Test case implementations must be serializable	Sealed class implements ITestCase but does not implement IXunitSerializable or [JsonTypeID]
xUnit3007	Warning	Test case implementations might not be serializable	Unsealed class implements ITestCase but does not implement IXunitSerializable or [JsonTypeID]

The distinction between X3006 and X3007 exists because unsealed classes might be handled by an external IXunitSerializer registration or a serializable subclass, which cannot be determined at compile time.

Behavior

• v2: ITestCase inherits from IXunitSerializable, so all implementors are inherently serializable. The analyzer correctly does not trigger for v2 projects.
• v3 (non-AOT): ITestCase does not inherit from IXunitSerializable. The analyzer triggers when a concrete class implements ITestCase without:
  • Implementing IXunitSerializable, or
  • Being decorated with [JsonTypeID]

[bradwilson]

Thanks! I made a few changes:

• [JsonTypeID] is not supported for test case serialization
• I updated the code to test for registered IXunitSerializer support
• I combined the tests (for performance reasons, we go for as few tests as possible)

[bradwilson]

I don't know what the stack overflow is that cause the macOS build to fail. I don't see any xUnit.net Analyzer code in the stack. ☹️ I'm going to merge anyway.

[bradwilson]

Thanks!

[ana1250]

Thanks for the refinements and for pushing the merge through, Brad!

That StackOverflowException makes sense now,I looked at the trace and it’s hitting WeightedMatch.MatchRecursive in the Microsoft testing library. It looks like the combination of the consolidated tests and the 19 nested markers in the CS0535 helper created a diagnostic matching tree that was just too deep for the macOS runner's stack and its capacity is less in compare to Windows /Linux?

Also, thanks for the clarification on [JsonTypeID] support for test cases and the update to the IXunitSerializer logic. Glad I could help.

[bradwilson]

That StackOverflowException makes sense now, I looked at the trace and it’s hitting WeightedMatch.MatchRecursive in the Microsoft testing library.

That's disappointing to hear. It sounds like I'm going to end up removing macOS from the CI test suite until it gets resolved.

Did you find an open issue for this already?

[ana1250]

I took a look through dotnet/roslyn-sdk and dotnet/roslyn, and I couldn’t find an existing issue that tracks this specific StackOverflowException in WeightedMatch.MatchRecursive.

From what I can tell, this seems to be triggered by the matching algorithm when there are many ambiguous diagnostics. In this case, the nested CS0535 markers generate a large number of similar diagnostics, which leads to very deep recursion and ends up hitting the stack limit on macOS.

I opened an upstream issue (dotnet/roslyn-sdk#1253) to document this behavior so it can be tracked on the Roslyn side.

In the meantime, I’ll put together a follow-up PR to simplify these test cases by reducing the number of compiler diagnostics generated. That should make the tests stable across platforms and avoid the need to remove macOS from CI.
Raised : #208