Skip to content

test(sourcegen): parse fixtures as a real build in all generator test harnesses - #7319

Merged
thomhurst merged 1 commit into
mainfrom
issue-7190-generator-test-parse-options
Nov 1, 2026
Merged

thomhurst merged 1 commit into
mainfrom
issue-7190-generator-test-parse-options

Conversation

@thomhurst

@thomhurst thomhurst commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

Summary

#7189 fixed the Core generator harness so it parses fixtures as a real build does. This PR applies the same fix to the other generator test harnesses (#7190):

  • Shared helper. tests/SharedTestHelpers/GeneratorTestInput.cs holds the LanguageVersion.Preview parse options and the input check from test(sourcegen): parse snapshot fixtures as a real build does #7189 (ThrowIfInputCannotBeParsed: syntax errors, plus CS0182/CS0225/CS8652, the errors a too-old compiler reports for valid newer syntax). The Core harness now uses it in place of its private copy.

  • TUnit.Assertions.SourceGenerator.Tests. TestsBase parses all inputs, including additional sources, with Preview options (preprocessor symbols are kept), passes them to CSharpGeneratorDriver.Create, and fails on unparseable input. AssertionMethodGeneratorRunner, the compile-check helpers (CompileChecker, AssertionExtensionGeneratorTests, XmlDocsRegressionTests, GlobalNamespaceRegressionTests) and UnboundGenericAssertionFromTests.RunGenerator use the same options.

  • TUnit.Assertions.Should.SourceGenerator.Tests. RunGenerator parses with Preview options, passes them to the driver, and fails on unparseable input.

  • Roslyn host. Both Assertions test projects now host on Roslyn 4.14 (VersionOverride), as the Core test project does, because Roslyn 4.7 rejects collection expressions in attribute arguments even in preview mode.

  • TUnit.Mocks.SourceGenerator.Tests. It already parsed with Preview options and runs on Roslyn 4.12 through Microsoft.CodeAnalysis.CSharp.Workspaces. MemberDiscoveryTests now uses the shared options too, and the snapshot paths of SnapshotTestBase (RunGenerator and the compile-diagnostics path) fail on unparseable input. RunGeneratorForDiagnostics is left as it was, because diagnostic tests may feed in input that is invalid on purpose.

  • Regression test. GeneratorTestInputTests checks that newer syntax (a collection expression in an attribute argument, a params IEnumerable<T> parameter) passes the check on the hosted Roslyn, and that a syntax error fails it.

  • Unrelated net472 fix. AotConverterScanTests.SkipsConversionsBlockedByObsoleteError (added in fix(sourcegen): skip [Obsolete(error: true)] conversion operators in AotConverterGenerator #7317) failed on the Windows-only net472 leg with CS0518, because its record struct fixture needs IsExternalInit. It now adds ReferencesHelper.NetFrameworkPolyfills on .NET Framework, as other Core tests already do. CI runs only on Linux and macOS, so it never ran this leg.

No snapshot changed, so none of the existing fixtures was being bound incorrectly.

Left out of scope: TUnit.SourceGenerator.IncrementalTests pins Roslyn 4.7 on purpose, to test the Roslyn47 generator variant, and its fixtures use only simple syntax.

Validation

  • dotnet test on every TFM for TUnit.Core.SourceGenerator.Tests, TUnit.Assertions.SourceGenerator.Tests (net472 included), TUnit.Assertions.Should.SourceGenerator.Tests and TUnit.Mocks.SourceGenerator.Tests: all pass, and no snapshot files changed.

Closes #7190

Summary by CodeRabbit

  • Tests
    • Expanded source-generator test coverage for newer C# syntax and malformed inputs.
    • Standardized parsing and validation across generator test suites, improving consistency in test results.
    • Added coverage for framework-specific compilation scenarios.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Walkthrough

The change adds a shared helper for preview-language parsing and input compilation validation. Assertions, Core, and Mocks source-generator test harnesses adopt the helper. The test projects include the shared source, and two Assertions test projects pin Roslyn packages to version 4.14.0.

Changes: Source-generator test harnesses

Layer / File(s) Summary
Shared parsing and validation
tests/SharedTestHelpers/GeneratorTestInput.cs, tests/TUnit.Assertions.SourceGenerator.Tests/GeneratorTestInputTests.cs
GeneratorTestInput provides preview parse options, source parsing, and input compilation validation. Tests cover newer syntax acceptance and rejection of syntax errors.
Assertions harness integration
tests/TUnit.Assertions.SourceGenerator.Tests/*, tests/TUnit.Assertions.Should.SourceGenerator.Tests/*
The test harnesses use shared parsing options and parsing. They validate input before running generators. The two test projects include the helper and pin the specified Roslyn packages to version 4.14.0.
Core harness integration
tests/TUnit.Core.SourceGenerator.Tests/*
The Core harness uses shared parsing options and input validation. Its project includes the helper, and the .NET Framework test compilation includes polyfills.
Mocks harness integration
tests/TUnit.Mocks.SourceGenerator.Tests/*
The Mocks harness uses shared parsing options and validates input before generator execution. Member discovery tests parse source through the helper, and the project includes it.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant TestHarness
  participant GeneratorTestInput
  participant Compilation
  participant GeneratorDriver
  TestHarness->>GeneratorTestInput: Parse source with preview options
  TestHarness->>Compilation: Build input compilation
  TestHarness->>GeneratorTestInput: Validate input compilation
  TestHarness->>GeneratorDriver: Run generator with preview parse options
Loading

Suggested reviewers: quattordici

Merge Risk: Minimal · up to 2db8b

The change improves fixture validation, though two existing runner gaps remain. No new merge-blocking risk is established.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

A rabbit checks each syntax tree,
Preview leaves the parse set free.
Errors hop out before the run,
Shared helpers guide the work begun.
The generators greet the spring.

@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-01T14:45:56.456641Z 2db8bc5 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Review

This is a well-scoped test-infrastructure change. A shared GeneratorTestInput replaces the per-harness parse options, and the generator driver and compilation now use the same Preview parse options. Unparseable input now fails the test instead of being snapshotted. That fixes the silent-wrong-snapshot class of bug from #7162.

Checked:

  • UnboundGenericAssertionFromTests already has its own ParseOptions field, so there is no naming clash with the shared helper.
  • Generated trees get the same parse options as the driver. The preprocessor-symbol handling in TestsBase is preserved.
  • RunGeneratorForDiagnostics is deliberately left lenient because diagnostic tests may feed invalid input on purpose. That is reasonable.
  • The regression tests cover both the accept and reject paths.

Optional suggestions, not blockers:

  1. UnsupportedSyntaxDiagnosticIds is a hand-maintained allowlist (CS0182, CS0225, CS8652). If a newer feature trips another binding error on an old host Roslyn, it will slip through again. The Roslyn 4.14 pin reduces this risk, but a comment tying the list to the hosted Roslyn version would help whoever next bumps it.
  2. The Microsoft.CodeAnalysis.* VersionOverride="4.14.0" entries are now duplicated in two csproj files. Moving them into a shared props file or a central property would keep them from drifting.
  3. The net472 IsExternalInit polyfill fix is unrelated to the harness change. It is disclosed in the description, which is fine.

I have not run the tests myself. The PR description reports that all TFMs pass with no snapshot changes. LGTM.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 2db8bc5302

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@greptile-apps

greptile-apps Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Low impact] The changes stay within source-generator test infrastructure.

Fix the missing System import in the added polyfill tree before merging.

Findings

  1. P1 Windows test still fails ▶

Summary

Shares Preview parse options and input checks across the generator test harnesses.

  • Assertion and Core generator tests parse fixtures like real builds do.
  • Mock generator snapshot tests check fixture input before running.
  • The AOT converter test supplies framework polyfills for its record fixture.

Reviews (1) · Last reviewed commit: "test(sourcegen): parse generator test fi..." · Reviewed by Greptile

@thomhurst

Copy link
Copy Markdown
Owner Author

Dispositions for head 2db8bc5:

  • Claude, suggestion 1 (comment tying the UnsupportedSyntaxDiagnosticIds allowlist to the hosted Roslyn version): a good idea, but it is not blocking. I left it out so the approved head does not change. It is worth adding the next time this file is edited.
  • Claude, suggestion 2 (shared VersionOverride for Roslyn 4.14): not blocking. These are two test-only projects with a pin that rarely changes.
  • Claude, suggestion 3: acknowledged. The net472 fix stays in this PR because it turns main's red Windows leg (run 41311511530) green. test: fix ClassDataSourceTupleTests to exercise class-level tuple unwrapping #7318 is blocked on it.
  • Greptile P1 (polyfill missing using System): not a bug. I replied in the thread with the reason and a local net472 pass, then resolved it.

@thomhurst
thomhurst marked this pull request as ready for review November 1, 2026 14:44
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Review

This is a sound test-infrastructure change. I read the diff but did not build the projects or run the tests.

What I checked

  • Moving the Preview parse options and ThrowIfInputCannotBeParsed into tests/SharedTestHelpers/GeneratorTestInput.cs removes the duplicated private copy in the Core harness. All the harnesses now parse the same way.
  • In the Assertions TestsBase, the parse options are now built before the driver is created. The driver and the additional-source trees use the same options, including the preprocessor symbols. Without that, the generated trees would use different options from the inputs.
  • Passing parseOptions into CSharpGeneratorDriver.Create and failing on unparseable input should stop snapshots from silently recording output for broken trees.
  • The #if NETFRAMEWORK polyfill in AotConverterScanTests is a sensible fix for IsExternalInit.
  • GeneratorTestInputTests covers both the accepted case (newer syntax) and the rejected case (syntax errors).

Suggestions (non-blocking)

  1. Roslyn version pin. VersionOverride="4.14.0" is now repeated in two csproj files, three packages each. Define a single $(TestHostRoslynVersion) property, or add it to Directory.Packages.props, so the Assertions and Core test projects can't drift apart.
  2. Diagnostic allow-list. UnsupportedSyntaxDiagnosticIds (CS0182, CS0225, CS8652) is a hand-maintained allow-list. A future too-old-compiler error with a different ID would still slip through. The list's comment says why each ID is there, which helps. The regression test could also assert that each ID is actually reported when the compiler is too old, so the list is checked rather than just trusted.
  3. ThrowIfInputCannotBeParsed in RunGeneratorForDiagnostics. Leaving it out for the Mocks diagnostics path is reasonable. A short comment at that call site would stop someone later "fixing" the inconsistency.

I don't see correctness problems. Approving in spirit.

@thomhurst
thomhurst enabled auto-merge (squash) November 1, 2026 14:46
@thomhurst
thomhurst merged commit 3847fb7 into main Nov 1, 2026
32 of 33 checks passed
@thomhurst
thomhurst deleted the issue-7190-generator-test-parse-options branch November 1, 2026 14:53

This branch was previously deployed

1 inactive deployment
Pull Requests — 2db8bc53 Deployed Nov 1, 2026 by thomhurst via modularpipeline (ubuntu-latest) #20498
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.

Other source-generator test harnesses parse fixtures with Roslyn 4.7 default language version

1 participant