Skip to content

Add CulturedConditionalFactAttribute and CulturedConditionalTheoryAttribute to XUnitV3Extensions#16825

Open
Copilot wants to merge 3 commits into
mainfrom
copilot/add-cultured-conditional-attributes
Open

Add CulturedConditionalFactAttribute and CulturedConditionalTheoryAttribute to XUnitV3Extensions#16825
Copilot wants to merge 3 commits into
mainfrom
copilot/add-cultured-conditional-attributes

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

Adds conditional variants of CulturedFactAttribute and CulturedTheoryAttribute that support CalleeType and ConditionMemberNames, matching the pattern of the existing ConditionalFact/ConditionalTheory attributes.

These derive from CulturedFactAttribute/CulturedTheoryAttribute (xunit v3 only) and use the existing ConditionalTestDiscoverer.EvaluateSkipConditions to set Skip at construction time.

  • New attributes in Microsoft.DotNet.XUnitV3Extensions/src/:
    • CulturedConditionalFactAttribute — extends CulturedFactAttribute
    • CulturedConditionalTheoryAttribute — extends CulturedTheoryAttribute
  • Tests in CulturedConditionalAttributeTests.cs: skip state validation, property exposure (CalleeType, ConditionMemberNames, Cultures), theory data passthrough, culture verification via CultureInfo.CurrentCulture
[CulturedConditionalFact(new[] { "en-US", "fr-FR" }, typeof(MyTests), nameof(IsSupported))]
public void RunsPerCultureWhenConditionMet() { }

[CulturedConditionalTheory(new[] { "en-US", "ja-JP" }, typeof(MyTests), nameof(IsSupported))]
[InlineData(42)]
public void TheoryRunsPerCultureWhenConditionMet(int value) { }

To double check:

Copilot AI and others added 2 commits May 16, 2026 18:45
Agent-Logs-Url: https://github.com/dotnet/arcade/sessions/d4c6180b-97a0-40ce-b44f-563c59ba63bd

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds xUnit v3 cultured conditional test attributes so tests can combine culture-specific execution with existing conditional skip evaluation.

Changes:

  • Adds CulturedConditionalFactAttribute.
  • Adds CulturedConditionalTheoryAttribute.
  • Adds tests validating skip behavior, exposed properties, theory data passthrough, and culture selection.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/Microsoft.DotNet.XUnitV3Extensions/src/CulturedConditionalFactAttribute.cs Adds the cultured conditional fact attribute implementation.
src/Microsoft.DotNet.XUnitV3Extensions/src/CulturedConditionalTheoryAttribute.cs Adds the cultured conditional theory attribute implementation.
src/Microsoft.DotNet.XUnitV3Extensions/tests/CulturedConditionalAttributeTests.cs Adds xUnit v3 tests covering the new attributes.

Comment thread src/Microsoft.DotNet.XUnitV3Extensions/tests/CulturedConditionalAttributeTests.cs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@AndriySvyryd AndriySvyryd marked this pull request as ready for review May 17, 2026 21:58
public class CulturedConditionalAttributeTests
{
// These tests validate the xunit v3 cultured conditional attributes without relying on
// execution order, which the v3 runner does not guarantee for this scenario.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndriySvyryd won't this potentially cause issues if the tests run concurrently with other tests and the process culture is changed?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be an issue in XUnit.
https://github.com/xunit/xunit/blob/main/src/xunit.v3.core/ObjectModel/CulturedXunitTestCase.cs#L103 sets it globally, but in V2 it was set only for a specific thread:
https://github.com/xunit/xunit/blob/main/src/xunit.v2.tests/TestUtility/CultureAwareTesting/CulturedXunitTestCase.cs#L53
This looks like a deliberate change, so there must be some mechanism to isolate the other tests.
@bradwilson Would you like to weigh it?

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.

4 participants