[thomhurst]

Reflection discovery invoked generated dynamic-test sources and then invoked the same builder methods again. It now skips TUnit-generated wrappers while preserving explicitly registered custom sources, invokes builders through reflection, and reads creator locations from DynamicTestBuilderAttribute for both successful and failed discovery. The type scan includes internal classes, consistent with ordinary reflection discovery.

The existing five-case dynamic-index regression checks that the builder runs exactly once per discovery and that each test retains its builder source location. Its counter resets before each discovery so host reuse does not cause false failures. A registered custom-source regression verifies discovery without a builder attribute in both modes. Also corrects the local AOT publishing command: the project uses Aot=true to avoid applying PublishAot to netstandard analyzer references.

Closes #7349

Validation:

• Release net10.0 test-application build passed (SDK 10.0.403).
• DynamicTestIndexTests: 5/5 passed in generated mode and 5/5 in reflection mode.
• DynamicTestInheritedAttributesTests (static builder): 3/3 passed in each mode.
• git diff --check passed.
• Native AOT publish with -p:Aot=true --use-current-runtime reached native linking but failed with ld: library 'ssl' not found on this macOS arm64 machine. Full AOT validation remains for CI. Dynamic-test APIs already declare their reflection/AOT limitations.
• Engine tests were left to CI as required by the repository guide. No generator output or public API changes.

Review fixes (110401b): Release net10.0 build passed; DynamicTestIndexTests 5/5 and RegisteredDynamicTestSourceTests 1/1 passed separately in generated and reflection modes. git diff --check passed. Engine tests remain for CI; AOT publishing was not repeated in this review cycle.

Review fix (411189a): preserve internal builder constructors in reflection mode. New internal-constructor regression passed 1/1 in generated mode and 1/1 in reflection mode. Release net10.0 build and Native AOT publish (-p:Aot=true --use-current-runtime, macOS arm64) passed; publishing reports existing trimming warnings. No generator output or public API changes. Engine tests remain for CI.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

Learn how review limits work.

Review configuration:

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: ab47d646-3043-468e-b6c0-292a8f052e7a

Commits

Reviewing files that changed from the base of the PR and between d8badac and 411189a.

Files selected for processing: 5

• .claude/docs/workflows.md
• src/TUnit.Engine/Discovery/ReflectionTestDataCollector.cs
• tests/TUnit.TestProject/DynamicTests/DynamicTestIndexTests.cs
• tests/TUnit.TestProject/DynamicTests/InternalConstructorDynamicBuilderTests.cs
• tests/TUnit.TestProject/DynamicTests/RegisteredDynamicTestSourceTests.cs

• 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

• X
• Mastodon
• Reddit
• LinkedIn

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

[thomhurst]

@codex review

[chatgpt-codex-connector]

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-03T01:10:04.044175Z	411189a	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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1d10597bbc

ℹ️ 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]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new blocking issue was found.

Summary

Reflection discovery skips TUnit-generated wrappers, preserves custom sources, and reads builder source locations from DynamicTestBuilderAttribute.

• Reflection discovery runs each dynamic builder once and keeps custom sources.
• Dynamic tests keep the file and line of their builder.
• The AOT guide uses the test project's scoped publish switch.

No new actionable issues were found.

_{Reviews (3) · Last reviewed commit: "Fix internal dynamic builder constructio..." · Reviewed by Greptile}

[thomhurst]

Addressed both findings in 110401b: custom registered dynamic sources are preserved, and the invocation counter resets before discovery. Release net10.0 build passed; five builder cases and one explicit-source case passed separately in each discovery mode (12 successful executions). Engine tests and full AOT validation remain for CI as documented above.

[thomhurst]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 110401bc4c

ℹ️ 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".

[github-actions]

Review

The fix is sound. Generated wrappers emit [GeneratedCode("TUnit", ...)] (DynamicTestsGenerator.cs:108), so skipping them by that marker avoids the double builder invocation. Custom registered sources are still collected, and the new regression tests cover both cases. Reading File and Line straight from DynamicTestBuilderAttribute is simpler than the old extraction helpers. I found no previous review findings left unaddressed.

Non-blocking suggestions:

• Marker coupling. The check keys on the GeneratedCode tool string "TUnit". A user-written source that copies that attribute would be silently skipped. A dedicated internal marker attribute or interface on generated wrappers would be a more explicit contract. The current approach is acceptable given the narrow risk.
• GetTypes() failures. If GetTypes() throws ReflectionTypeLoadException, the catch discards the whole assembly. Using ex.Types.Where(t => t != null) would keep discovery working for partially loadable assemblies. This was already the behaviour before the PR, and the switch to GetTypes() slightly widens the exposure.
• Unused helpers. If ExtractFilePath and ExtractLineNumber now have no callers, they can be deleted.

Looks good to merge once CI is green.

[thomhurst]

Current-head review is clean from Claude and Codex, and both previous blocking threads are resolved. Disposition of Claude's non-blocking suggestions:

• Keep the existing GeneratedCode("TUnit", ...) marker for this fix: it matches the generator's emitted marker and preserves unmarked custom registrations. A dedicated marker would be a separate design change.
• Tracked the pre-existing catch-and-empty behavior for partial assembly type loading in Reflection dynamic discovery drops loadable builders when assembly type loading fails #7429; it does not block this duplicate-invocation fix.
• Keep ExtractFilePath and ExtractLineNumber: they still serve ordinary and failed-test metadata paths (including lines 1755–1756, 3120–3121, and 3195–3196 on this head).

Promoting this reviewed head to ready; CI and the ready-event Claude review will now run.

[thomhurst]

Marked ready with head unchanged at 110401b. Waiting for ready-event Claude review run 41552230993 and CI (including .NET run 41552230597). Auto-merge is not armed: repository guidance requires the ready-event Claude review to pass and its comment to be dispositioned first. Both required reviewers approved the draft head; prior threads are resolved.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 110401bc4c

ℹ️ 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".

[thomhurst]

@codex review

[thomhurst]

Validation for 411189a: Release net10.0 build passed; the internal-constructor regression passed 1/1 separately in generated and reflection modes; Native AOT publishing with -p:Aot=true --use-current-runtime passed on macOS arm64 (with trimming warnings); git diff --check passed. The constructor finding is fixed and its thread resolved. Returned to draft for fresh current-head Claude and Codex reviews. No CI wait or deferral is needed to report this pushed fix.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 411189afeb

ℹ️ 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".

[github-actions]

Review

The fix is sound. The generator's DynamicTestsGenerator (line 108) marks its wrappers with GeneratedCode("TUnit", ...), so the new skip logic in DiscoverDynamicTests does match them. Custom registered sources are still collected. The DynamicTestBuilderAttribute File/Line read is consistent for the success and failure paths. The regression tests cover once-only invocation, the custom-source case and internal constructors.

Suggestions, none blocking:

• Identifying generated sources by the GeneratedCode tool string is implicit coupling. A marker interface or a dedicated attribute on generated wrappers would be sturdier. A user type that happens to claim Tool == "TUnit" would be skipped silently. Adding a comment next to the generator's emit line that points at this check would help.
• Switching from GetExportedTypes to GetTypes widens the scan. It now covers internal and nested types in every scanned assembly. The catch around it falls back to nothing, so a ReflectionTypeLoadException drops the whole assembly. Using ex.Types.Where(t => t != null) in that fallback would be more resilient, and the file already has similar handling around line 383.
• Activator.CreateInstance(testClass, nonPublic: true) is not trim-annotated. The PR description says AOT publishing passed, and dynamic tests are already marked as reflection-only, so I'm fine with this.

Overall this looks good to merge once CI is green.

🤖 Reviewed with Claude Code

[thomhurst]

Run 41552230993 was cancelled on the previous head and is superseded by the successful current-head Claude review (41552797570). Claude and Codex both approve 411189a; all three review threads are resolved.

Current Claude suggestions are non-blocking: retain the existing generated-code marker contract for this fix, as previously dispositioned; partial assembly type loading remains tracked in #7429; retain the constructor reflection path given the documented dynamic-test reflection limitations and successful Native AOT publish. No additional change is required for promotion. Marking this reviewed head ready to run CI and the ready-event Claude review.