Fix XUnitWrapperGenerator: testArguments out of scope in ConditionalTest else branch for MemberData theories

[Copilot]

When a [Theory] with [MemberData] is also annotated with a conditional skip attribute ([SkipOnPlatform], [SkipOnCoreClr], etc.), the generator wraps the MemberDataTest inside a ConditionalTest. The else branch of that conditional reports the skipped test using MemberDataTest.TestNameExpression — which was previously copied verbatim from the inner BasicTestMethod.TestNameExpression. That expression references the testArguments foreach loop variable, which is only in scope inside the if branch, producing:

error CS0103: The name 'testArguments' does not exist in the current context

Fix (ITestInfo.cs — MemberDataTest constructor):

• Reorder assignments so Method/ContainingType are set first
• Replace TestNameExpression = innerTest.TestNameExpression with a static literal: $"\"{externAlias}::{ContainingType}.{Method}(...)\""
• The inner BasicTestMethod.TestNameExpression (used inside the foreach loop for per-iteration filter/reporting) is unchanged

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

Fixes a C# compile error in the XUnitWrapperGenerator output when [Theory] + [MemberData] tests are wrapped in a ConditionalTest (e.g., via [SkipOnPlatform]), by ensuring the else-branch skip-reporting name does not reference the foreach loop variable (testArguments) that’s only in scope in the if branch.

Changes:

• Updates MemberDataTest to use a static TestNameExpression literal (no loop variable reference).
• Reorders MemberDataTest constructor assignments so Method/ContainingType are set before building the test name.
• Adds an explanatory comment about the scope issue (ConditionalTest else branch vs foreach scope).

[danmoseley]

build analysis stuck

[github-actions]

🤖 Copilot Code Review — PR #126505

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6 primary, Claude Sonnet 4.6 + GPT-5.3-codex secondary).

Holistic Assessment

Motivation: The bug is real and clear. When ConditionalTest wraps a MemberDataTest, the generated else branch references testArguments — a loop variable only in scope inside the foreach in the if branch — causing CS0103. This means any [Theory] with [MemberData] combined with conditional skip attributes ([SkipOnPlatform], [SkipOnCoreClr], etc.) produces uncompilable test wrappers.

Approach: The fix is minimal and correct. Changing MemberDataTest.TestNameExpression to a static literal avoids the scope issue while preserving per-argument reporting inside the foreach loop via the inner BasicTestMethod.TestNameExpression. Alternative approaches (moving the foreach to encompass both branches, special-casing in ConditionalTest, adding a separate OutOfScopeTestNameExpression) would all be more complex for no added benefit.

Summary: ✅ LGTM. The fix correctly resolves a compile-time error in generated code with a focused 3-line change. All code paths were traced and verified correct. No new issues introduced. All three models (Claude Opus 4.6, Claude Sonnet 4.6, GPT-5.3-codex) agree on the verdict.

---

Detailed Findings

✅ Correctness — Fix resolves the CS0103 scope error

Traced the full code-generation flow:

Generated code site	TestNameExpression source	Inside foreach?	Valid?
BasicTestMethod → WrapTestExecutionWithReporting(..., this)	BasicTestMethod (dynamic, refs testArguments)	✅ Yes	✅
ConditionalTest else → GenerateSkippedTestReporting(_innerTest)	MemberDataTest.TestNameExpression	❌ No	✅ Fixed by this PR
WrapperLibraryTestSummaryReporting filter/report calls	BasicTestMethod.TestNameExpression (passed as this inside loop)	✅ Yes	✅

MemberDataTest.TestNameExpression is never referenced inside the foreach loop body — MemberDataTest.GenerateTestExecution() delegates entirely to _innerTest.GenerateTestExecution(wrapper), where _innerTest is the BasicTestMethod carrying the per-argument interpolated expression. The MemberDataTest.TestNameExpression is only exposed to outer wrappers (ConditionalTest), where the static literal is both correct and necessary.

✅ Property assignment reordering is correct

Moving Method and ContainingType assignments before TestNameExpression is necessary because the new expression uses these values. The original order only worked because it forwarded innerTest.TestNameExpression verbatim.

✅ Equality/hashing behavior preserved (flagged by all 3 models)

MemberDataTest.Equals checks TestNameExpression, Method, ContainingType, and _innerTest.Equals(...). Previously TestNameExpression mirrored _innerTest.TestNameExpression exactly, making it redundant in Equals. Now the two diverge, but _innerTest.Equals still provides full distinctiveness via BasicTestMethod's per-argument expressions. No regression.

ConditionalTest copies TestNameExpression from its inner test (line 143). After this fix, it gets the static string — correct since ConditionalTest should never reference loop variables either.

💡 Minor test-name fidelity change in skip reporting — acceptable (flagged by all 3 models)

When a ConditionalTest(MemberDataTest) skips (condition is false), the skip report now shows "alias::Type.Method(...)" instead of per-argument details. This is a non-issue because: (1) the old code never compiled, so this path was always broken; (2) when the condition is false, the entire member-data loop is skipped — there are no specific arguments to report.

💡 No regression test — acceptable for this context

The XUnitWrapperGenerator has no dedicated unit test suite. The "test" is whether projects using [Theory] + [MemberData] + conditional skip attributes compile successfully, which CI will verify. A dedicated generator test would be ideal but is out of scope for this fix.

💡 Pre-existing: _memberInvocation not included in Equals/GetHashCode (not introduced by this PR)

Two MemberDataTest instances wrapping the same BasicTestMethod but backed by different member data sources would compare as equal. This is a pre-existing gap — noting for awareness, not a PR blocker.

Generated by Code Review for issue #126505 · ◷