[thomhurst]

Summary

ClassDataSourceTupleTests always failed with InvalidCastException. [ClassDataSource<TupleDataSource>] creates an instance of TupleDataSource and injects that instance, so the constructor received the data-source object and GetDataRowsAsync never ran.

• TupleDataSource now derives from Attribute and is applied directly as [TupleDataSource], so its tuple rows reach the constructor and class-level tuple unwrapping is actually tested.
• ClassDataSourceTupleTests and StaticMethodClassDataSourceTupleTests now carry [EngineTest(ExpectedResult.Pass)] so CI runs them.

Validation

dotnet test -f net10.0 in tests/TUnit.TestProject, in source-generated and --reflection mode:

• /*/TUnit.TestProject/ClassDataSourceTupleTests/*: 3/3 passed in both modes
• /*/TUnit.TestProject/StaticMethodClassDataSourceTupleTests/*: 1/1 passed in both modes

Closes #7184

Summary by CodeRabbit

• Tests
  • Updated test coverage for tuple-based class data sources, including constructor-parameter scenarios and expected passing results.

[coderabbitai]

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: efa68d14-c442-46ec-b47a-8b204db42741

Commits

Reviewing files that changed from the base of the PR and between 3847fb7 and 1db04e7.

Files selected for processing: 1

• tests/TUnit.TestProject/ClassDataSourceTupleTests.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 tuple test data source now works as a class-level attribute. Both tuple tests declare an expected passing result.

Changes: Tuple data-source test correction

Layer / File(s)	Summary
Attribute and tuple test coverage tests/TUnit.TestProject/ClassDataSourceTupleTests.cs	TupleDataSource now derives from Attribute and implements IDataSourceAttribute. The constructor-parameter test applies [TupleDataSource] directly. Both tuple tests declare [EngineTest(ExpectedResult.Pass)].

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

Change: Other · Severity of issue fixed: Low

Possibly related PRs

• thomhurst/TUnit#7319: This PR fixes a compilation failure reported as a blocker for this change, though it does not modify tuple unwrapping.

Merge Risk: Minimal · up to 1db04

The test changes align with the stated tuple data-source coverage and reported passing results.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and specifically describes the main change: updating ClassDataSourceTupleTests to exercise class-level tuple unwrapping.
Linked Issues check		Issue #7184 has one direct coding requirement set. The test must exercise class-level tuple unwrapping, pass, and carry [EngineTest(ExpectedResult.Pass)]. The reviewed head meets each requirement in t…
Out of Scope Changes check		The whole-PR diff touches one file: tests/TUnit.TestProject/ClassDataSourceTupleTests.cs (+9/-3). Each change connects to #7184 or to the same tuple-unwrapping test scenario: - TupleDataSource base ty…
Docstring Coverage		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…

Full details: Linked Issues check

Explanation

Issue #7184 has one direct coding requirement set. The test must exercise class-level tuple unwrapping, pass, and carry [EngineTest(ExpectedResult.Pass)]. The reviewed head meets each requirement in tests/TUnit.TestProject/ClassDataSourceTupleTests.cs (diff against merge base 3847fb7):

• TupleDataSource now derives from Attribute and implements IDataSourceAttribute. The class carries [AttributeUsage(AttributeTargets.Class)].
• ClassDataSourceTupleTests uses [TupleDataSource] directly. The [ClassDataSource<TupleDataSource>] usage is removed, so the attribute instance is no longer injected in place of the tuple rows.
• ClassDataSourceTupleTests has [EngineTest(ExpectedResult.Pass)]. EngineTest is defined in tests/TUnit.TestProject/Attributes/ExpectedPassEngineTest.cs, and the file adds the required using.
  The test body is unchanged. The pass result is supported by the author's reported local runs (source-generated and --reflection, net10.0, 3/3). I did not execute the tests, so the runtime pass is author-reported evidence.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff touches one file: tests/TUnit.TestProject/ClassDataSourceTupleTests.cs (+9/-3). Each change connects to #7184 or to the same tuple-unwrapping test scenario:

• TupleDataSource base type and AttributeUsage change: required so that [TupleDataSource] can be applied directly (issue Expected section).
• [TupleDataSource] replaces [ClassDataSource<TupleDataSource>] on ClassDataSourceTupleTests: directly requested by #7184.
• [EngineTest(ExpectedResult.Pass)] on ClassDataSourceTupleTests: directly requested by #7184.
• [EngineTest(ExpectedResult.Pass)] on StaticMethodClassDataSourceTupleTests: not named in #7184. It is a sibling test in the same file that exercises the same tuple-unwrapping path. The PR description and the PR objectives state this change explicitly. It enables CI for the same test category and does not alter unrelated behavior.
  No unrelated files or refactors appear in the diff. Pipeline, rebase, and rerun events in the PR comments are not code changes.

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 1 files.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• 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

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the tuple row,
Then sees the class attribute flow.
Two tests now state the pass they seek,
Their expected results clear and neat.
The rabbit hops through green-lit code.}

[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-01T17:26:11.487156Z	1db04e7	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]

Review

Looks good. The fix is correct: [ClassDataSource<T>] instantiates T and injects the instance, so TupleDataSource as a constructor argument source never ran GetDataRowsAsync. Making it an Attribute and applying it directly exercises the real class-level tuple unwrapping. The explanatory comment is helpful, and [EngineTest(ExpectedResult.Pass)] on both classes means CI now runs them. This is a test-only change. I found no other references to TupleDataSource outside the unrelated analyzer and generator test names, so nothing else breaks.

Two optional notes:

• Check that TUnit.TestProject.Attributes is the right namespace for EngineTest in this project. The PR description reports the tests passing in both source-gen and reflection modes, which suggests it is.
• Consider renaming TupleDataSource to TupleDataSourceAttribute, following the convention used elsewhere. This is not required.

No blocking issues.

[chatgpt-codex-connector]

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

Reviewed commit: 4b562bbb59

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

[Low impact] The PR appears safe to merge.

Summary

Fixes the tuple fixture so its data rows reach the constructor.

• Class-level tuple tests now get data from their data source.

_{Reviews (2) · Last reviewed commit: "test: exercise class-level tuple unwrapp..." · Reviewed by Greptile}

[thomhurst]

Both reviewers approved head 4b562bb (Claude: no blockers; Codex: no findings). Greptile also found nothing.

Update: blocker #7319 has merged (3847fb799d), which fixes the net472 CS0518 IsExternalInit failure in AotConverterScanTests.SkipsConversionsBlockedByObsoleteError that turned modularpipeline (windows-latest) red. I rebased this PR onto origin/main and pushed it as 1db04e7 so Windows CI runs fresh. The PR's own diff did not change. After the rebase, ClassDataSourceTupleTests (source-gen mode, 3/3) and StaticMethodClassDataSourceTupleTests (reflection mode, 1/1) pass locally on net10.0. The new head needs fresh Claude and Codex verdicts before this PR is marked ready.

Optional notes from the Claude review:

• EngineTest namespace: it is correct. The tests ran under [EngineTest] in both modes.
• Renaming TupleDataSource to TupleDataSourceAttribute: not done. The class is a test-local fixture, and the shorter name matches how the test applies it.

[thomhurst]

@codex review

[github-actions]

Review

This is a small, test-only fix, and I think it's correct.

• Root cause: [ClassDataSource<TupleDataSource>] creates and injects a TupleDataSource instance. The constructor therefore got the data-source object, not its rows, which explains the InvalidCastException. Making TupleDataSource an Attribute and applying it directly fixes this, and it exercises the tuple-unwrapping path the test was meant to cover.
• CI coverage: Adding [EngineTest(ExpectedResult.Pass)] to both classes means these tests now run in CI. You said you ran them in both source-generated and reflection modes, and all passed.
• Attribute usage: AttributeUsage(Class) is appropriate here.

I have no blocking concerns.

Optional: rename TupleDataSource to TupleDataSourceAttribute. That would match the usual attribute naming and make it clear it isn't a plain class for ClassDataSource<T>. The existing comment already explains the intent, so this isn't necessary.

LGTM.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 1db04e787e

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

Status: the earlier blocker #7319 has merged. Both reviewers approved the current head 1db04e7: the Claude review found no blockers, and the Codex receipt for 1db04e787e found no issues. modularpipeline (windows-latest) now passes.

modularpipeline (macos-latest) in run 41320771706 was killed with exit code 137 (SIGKILL) about 2.5 minutes into the pipeline, right after AddLocalNuGetRepositoryModule. No test module had reported a failure, so this looks like a runner kill rather than a problem with this test-only diff. I reran the failed job once; this PR has no reruns left after it. Once macOS passes, the PR will be marked ready and auto-merge armed.

[github-actions]

Review

Small, test-only change. The diagnosis is correct: [ClassDataSource<TupleDataSource>] instantiates the type and injects the instance, so the constructor got the data-source object and GetDataRowsAsync never ran. Making TupleDataSource an Attribute and applying it directly fixes this, so the tuple unwrapping path is now exercised.

• [AttributeUsage(AttributeTargets.Class)] fits how it is used.
• Adding [EngineTest(ExpectedResult.Pass)] to both classes means CI now runs them. This is where the earlier silent failure came from.
• Nothing else in the repo references TupleDataSource from TUnit.TestProject. The other matches are unrelated generator or analyzer test names.
• The PR description reports 3/3 and 1/1 passing in both source-gen and reflection modes. I did not re-run them.

Optional: if ClassDataSource<T> with tuple-returning data is meant to be supported, a separate test could cover that. I don't think it is, since ClassDataSource is documented as injecting instances.

LGTM.