Skip to content

fix; Method output json-schema required state#22

Merged
IvanMurzak merged 13 commits intomainfrom
fix/return-schema-required
Nov 9, 2025
Merged

fix; Method output json-schema required state#22
IvanMurzak merged 13 commits intomainfrom
fix/return-schema-required

Conversation

@IvanMurzak
Copy link
Copy Markdown
Owner

This pull request updates the project to support both .NET 8 and .NET 9 across all test and main projects, and improves test coverage for schema generation by parameterizing tests to account for differences in nullability detection between frameworks. Additionally, it refines test assertions for required schema fields.

Multi-targeting and Framework Support

  • Updated ReflectorNet.Tests.OuterAssembly.csproj, ReflectorNet.Tests.csproj, and ReflectorNet/ReflectorNet.csproj to target both net8.0 and net9.0, ensuring compatibility and test coverage for both frameworks. Also, the main project now uses LangVersion 10.0 for broader compatibility. [1] [2] [3]

Test Improvements for Nullability and Required Fields

  • Converted several [Fact] tests in ReturnSchemaTests.cs to [Theory] with [InlineData], allowing tests to dynamically check for required fields based on the target framework's nullability detection capabilities. This includes tests for nullable custom types, complex lists, and wrapper classes. [1] [2] [3] [4]
  • Improved assertion logic in SchemaTestBase.cs to fail explicitly if the schema does not contain a required array, making test failures clearer and more actionable.

@IvanMurzak IvanMurzak requested a review from Copilot November 9, 2025 03:05
@IvanMurzak IvanMurzak self-assigned this Nov 9, 2025
@IvanMurzak IvanMurzak added the bug Something isn't working label Nov 9, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for .NET 8.0 target framework and adjusts the C# language version to accommodate multi-targeting across netstandard2.1, net8.0, and net9.0. The changes include test adaptations to handle different nullability detection behaviors between framework versions.

  • Added net8.0 to target frameworks for the main library and test projects
  • Downgraded C# language version from 11.0 to 10.0 in the main library
  • Updated tests with conditional compilation to handle nullability detection differences across framework versions
  • Improved test diagnostics in SchemaTestBase

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ReflectorNet/ReflectorNet.csproj Added net8.0 target framework and downgraded language version to 10.0
ReflectorNet.Tests/ReflectorNet.Tests.csproj Added net8.0 target framework for tests
ReflectorNet.Tests.OuterAssembly/ReflectorNet.Tests.OuterAssembly.csproj Added net8.0 target framework for outer assembly tests
ReflectorNet.Tests/SchemaTests/SchemaTestBase.cs Added explicit failure case when required array is missing in AssertResultRequired
ReflectorNet.Tests/SchemaTests/ReturnSchemaTests.cs Added conditional compilation directives to handle nullability detection differences between NET5+ and older frameworks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ReflectorNet/ReflectorNet.csproj
@IvanMurzak IvanMurzak requested a review from Copilot November 9, 2025 05:10
@IvanMurzak IvanMurzak marked this pull request as ready for review November 9, 2025 05:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ReflectorNet/src/Utils/MethodUtils.cs Outdated
Comment thread ReflectorNet/ReflectorNet.csproj
@IvanMurzak IvanMurzak merged commit a2ec951 into main Nov 9, 2025
1 check passed
@IvanMurzak IvanMurzak deleted the fix/return-schema-required branch November 9, 2025 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants