Skip to content

test(sourcegen): snapshot property injection output for Core.* namespaces - #7293

Merged
thomhurst merged 1 commit into
mainfrom
test/property-injection-snapshot
Oct 30, 2026
Merged

thomhurst merged 1 commit into
mainfrom
test/property-injection-snapshot

Conversation

@thomhurst

@thomhurst thomhurst commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Description

Follow-up to #7291. The tests added there check that the property injection output compiles. This PR adds a snapshot test of the generated source, so a regression shows the exact lines that lose the global:: prefix.

  • New fixture tests/TUnit.TestProject/Bugs/_7291/Tests.cs in namespace Core.Api.Tests. It covers each site that fix(sourcegen): globally qualify type names in property injection output #7291 fixed: the containing type, the data source attribute type, a generic base with a constraint, an enum cast in an attribute argument, and a keyword-named enum (@event).
  • New snapshot test Tests7291 runs PropertyInjectionSourceGenerator on the fixture. TestsBase gets a PropertyInjectionGenerator runner for this.
  • The fixture is also a real test in TUnit.TestProject. It checks that each property is injected and initialized, so the real build and both engine modes cover it too.

Type of Change

  • Test only

Testing

  • Tests7291 passes on net10.0, net9.0, net8.0 and net472.
  • With the generator from before fix(sourcegen): globally qualify type names in property injection output #7291, the snapshot fails. The diff shows each unqualified name, for example typeof(Core.Api.Tests.Tests) and where TProgram : class, Core.Api.Tests.IMarker.
  • Full TUnit.Core.SourceGenerator.Tests on net10.0: 213 passed, 1 skipped.
  • TUnit.TestProject with --treenode-filter "/*/Core.Api.Tests/*/*": passes in source-generated and --reflection modes.

Summary by CodeRabbit

  • Tests
    • Expanded coverage of property injection across inherited and generic fixtures, including required properties and custom data sources with enum arguments.
    • Added checks that generated sources are present and that injected database properties are initialized and generated values match expectations.

…aces

Follow-up to #7291. The existing tests only check that the generated
property injection code compiles. A snapshot of the output shows exactly
which type names lose their global:: prefix if the generator regresses.

The fixture lives in TUnit.TestProject under namespace Core.Api.Tests, so
the real build and both engine modes also cover it.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 30, 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-10-30T16:15:48.192407Z ca52998 PR opened
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 98efaf7d-7e26-4630-81ab-47e5cd25a90a
Commits

Reviewing files that changed from the base of the PR and between d51080f and ca52998.

Files selected for processing: 4
  • tests/TUnit.Core.SourceGenerator.Tests/Bugs/7291/Tests7291.cs
  • tests/TUnit.Core.SourceGenerator.Tests/Tests7291.Test.verified.txt
  • tests/TUnit.Core.SourceGenerator.Tests/TestsBase.cs
  • tests/TUnit.TestProject/Bugs/_7291/Tests.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

The change adds an issue 7291 fixture for property injection on concrete and generic-base properties. The fixture checks database initialization and enum-backed data. A source-generator test checks that generated files are present and records the expected generated sources.

Changes: Property injection regression coverage

Layer / File(s) Summary
Regression fixture and assertions
tests/TUnit.TestProject/Bugs/_7291/Tests.cs
Adds initialized database properties on the test class and its generic base class. The test checks initialization and verifies that the enum-backed value is "B-B".
Generator test and expected output
tests/TUnit.Core.SourceGenerator.Tests/Bugs/7291/Tests7291.cs, tests/TUnit.Core.SourceGenerator.Tests/TestsBase.cs, tests/TUnit.Core.SourceGenerator.Tests/Tests7291.Test.verified.txt
Adds a test that runs PropertyInjectionGenerator on the fixture and checks for generated files. Adds expected generated sources for the concrete and generic-base properties.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: Minimal · up to ca529

The change adds regression coverage without an identified merge-blocking issue.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the main change: adding a source-generator snapshot test for property-injection output in Core.* namespaces.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 3 files. (1 skipped: 1 unsupported.)

Finishing Touches: 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • 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 property in place,
Then watches generated sources trace.
Two databases wake and glow,
Enum-made values say “B-B” so.
The test confirms the sources show.

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Review

Test-only PR, and it looks good.

  • The snapshot fixture lives in Core.Api.Tests, so it reproduces the TUnit.Core.* name-binding problem from fix(sourcegen): globally qualify type names in property injection output #7291. It exercises each fixed site: the containing type, the data source attribute, a generic base type argument with a constraint, an enum cast, and the keyword-named @event enum.
  • The verified output has global:: on every type name, including the generic accessor constraint (where TProgram : class, global::Core.Api.Tests.IMarker).
  • The fixture also runs as a real engine test, so both source-generated and reflection modes cover it, and it breaks the build if the generated code stops compiling.
  • The PropertyInjectionGenerator runner in TestsBase follows the existing pattern, and the verified file sits at the test-project root next to the others.

One non-blocking note: the verified snapshot has no #if NET9_0_OR_GREATER variance by target framework. The PR says it passes on net8, net9, net10 and net472, so that's fine as is. Hashes like dc251b93 are stable because they come from the type name.

I did not run the tests myself, and I'm relying on the PR's stated results. Nothing here needs changing.

@greptile-apps

greptile-apps Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable issues were found.

Summary

Adds a snapshot test for property injection in Core.Api.Tests.

  • A Core-namespaced fixture checks generated property-injection names and values.

No actionable issues were found. Tests were not run during this review.

Reviews (1) · Last reviewed commit: "test(sourcegen): snapshot property injec..." · Reviewed by Greptile

@thomhurst
thomhurst enabled auto-merge (squash) October 30, 2026 16:17
@thomhurst
thomhurst merged commit 37c3fb4 into main Oct 30, 2026
25 of 26 checks passed
@thomhurst
thomhurst deleted the test/property-injection-snapshot branch October 30, 2026 17:05
thomhurst added a commit that referenced this pull request Oct 31, 2026
NamespaceStartingWithCore_GeneratedCodeCompiles (#7293) failed on the
Windows net472 leg: .NET Framework lacks IsExternalInit and
RequiredMemberAttribute for the source's init/required members, and has no
UnsafeAccessor for the NET8_0_OR_GREATER branches. Share the existing
polyfill source through ReferencesHelper and compile only the reflection
fallback branch on net472.
thomhurst added a commit that referenced this pull request Oct 31, 2026
…7296)

* feat(engine): report unobserved task exceptions in session results

TaskScheduler.UnobservedTaskException was only written to stderr, so the
run stayed green and the exceptions never reached any report. Record them
on TestSessionContext.UnobservedTaskExceptions and report them at session
end: a console warning, an HTML Overview panel, and the JUnit suite-level
system-err.

--fail-on-unobserved-task-exceptions (or
Settings.Execution.FailOnUnobservedTaskExceptions) forces a final GC so late
faults are not lost, and reports each exception as a failed result, which
reaches TRX and the exit code.

The event fires on the finalizer thread, which has no AsyncLocal contexts,
so a static tracker routes it to the running sessions.

Closes #7288

* test(sourcegen): compile raw property-injection tests on net472

NamespaceStartingWithCore_GeneratedCodeCompiles (#7293) failed on the
Windows net472 leg: .NET Framework lacks IsExternalInit and
RequiredMemberAttribute for the source's init/required members, and has no
UnsafeAccessor for the NET8_0_OR_GREATER branches. Share the existing
polyfill source through ReferencesHelper and compile only the reflection
fallback branch on net472.

* fix(engine): scope unobserved task exception reports per session

- Clear the HTML reporter's exception queue at session start, so a later
  IDE session does not report an earlier session's exceptions.
- Record each cause of an AggregateException separately, so reports keep
  every cause's type and stack trace.
- When several sessions run at once, report the exception in each but do
  not fail any of them, since the event names no session.

* fix(engine): keep empty AggregateException causes of unobserved tasks

AggregateException.Flatten() drops an empty AggregateException, so a task
that threw one recorded nothing and could not fail the run. Unwrap only
aggregates that wrap causes.
This was referenced Nov 2, 2026

This branch was previously deployed

1 inactive deployment
Pull Requests — ca529980 Deployed Oct 30, 2026 by thomhurst via modularpipeline (ubuntu-latest) #20451
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.

1 participant