Skip to content

Conversation

@TimothyMakkison
Copy link
Contributor

I've made the methods a little more generic so It can be used with different generators.

Follow up to #4563

@thomhurst
Copy link
Owner

Summary

Adds a new test project to verify incremental source generator behavior for the MethodAssertionGenerator.

Critical Issues

1. VSTest Dependency Violation (BLOCKING)
The new project uses xunit with xunit.runner.visualstudio and Microsoft.NET.Test.Sdk, which violates TUnit's critical rule #3: "No VSTest - Use Microsoft.Testing.Platform only. NEVER use Microsoft.VisualStudio.TestPlatform."

Location: TUnit.SourceGenerator.IncrementalTests/TUnit.SourceGenerator.IncrementalTests.csproj:15-16

Fix: Replace xunit with TUnit for the test framework. Look at existing test projects like TUnit.Assertions.SourceGenerator.Tests which correctly use TUnit, not xunit.

2. Missing Snapshot Test Updates
The PR adds incremental generator tests but doesn't include any .verified.txt snapshot files. Per rule #2: "Changes to source generator output or public APIs require running snapshot tests. Commit .verified.txt files. NEVER commit .received.txt."

While these are incremental behavior tests (not output tests), if any test failures produce .received.txt files, they should be reviewed and accepted as .verified.txt before merging.

3. Target Framework Inconsistency
The new project targets net10.0 while existing TUnit test projects use properties from TestProject.props. This creates inconsistency.

Location: TUnit.SourceGenerator.IncrementalTests/TUnit.SourceGenerator.IncrementalTests.csproj:6

Fix: Import TestProject.props at the top like other test projects:

<Import Project="../TestProject.props" />

Then remove the explicit TargetFramework setting.

Suggestions

  1. Consistency with existing tests: The project structure differs significantly from existing test projects. Consider following the pattern in TUnit.Assertions.SourceGenerator.Tests which uses TUnit and the standard test project structure.

  2. Use public API from MethodAssertionGenerator: The tests reference MethodAssertionGenerator.BuildAssertion (line 20). This is good - it's the named pipeline step. Just verify this is intentionally public.

  3. Consider adding documentation: Since this introduces a new testing pattern (incremental generator testing), adding a brief comment in TestHelper.cs explaining why this is a separate project could help maintainers.

Previous Review Status

Unable to retrieve previous review comments due to API scope limitations.

Verdict

⚠️ REQUEST CHANGES - Critical rule violations must be fixed (VSTest dependency, missing import of TestProject.props)

@TimothyMakkison TimothyMakkison force-pushed the method_assertion_incre_test branch from 48bb006 to 8eeeb4f Compare January 28, 2026 23:59
@TimothyMakkison
Copy link
Contributor Author

TimothyMakkison commented Jan 29, 2026

  1. Target Framework Inconsistency

SelfRegisteredExtensions.cs(15, 9): [CS0234] The type or namespace name 'Framework' does not exist in the namespace 'TUnit.Engine' (are you missing an assembly reference?)

  1. VSTest Dependency Violation (BLOCKING)

Tried switching to TUnit, not sure if they run was going to see if CI liked it

Updated the .sln file by blindly accepting everything, hopefully it's fine

@TimothyMakkison TimothyMakkison marked this pull request as draft January 29, 2026 00:15
@TimothyMakkison TimothyMakkison marked this pull request as ready for review January 29, 2026 00:29
@TimothyMakkison TimothyMakkison force-pushed the method_assertion_incre_test branch from 8eeeb4f to 7acfb9a Compare January 29, 2026 00:29
@TimothyMakkison
Copy link
Contributor Author

Can't get it to run with TUnit, I'm absolutely missing something here, but at least it runs 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants