Add CulturedConditionalFactAttribute and CulturedConditionalTheoryAttribute to XUnitV3Extensions#16825
Add CulturedConditionalFactAttribute and CulturedConditionalTheoryAttribute to XUnitV3Extensions#16825Copilot wants to merge 3 commits into
Conversation
Agent-Logs-Url: https://github.com/dotnet/arcade/sessions/d4c6180b-97a0-40ce-b44f-563c59ba63bd Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/arcade/sessions/0596fa58-4ded-403f-84c8-d8a179fb05c9 Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
There was a problem hiding this comment.
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. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| 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. |
There was a problem hiding this comment.
@AndriySvyryd won't this potentially cause issues if the tests run concurrently with other tests and the process culture is changed?
There was a problem hiding this comment.
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?
Adds conditional variants of
CulturedFactAttributeandCulturedTheoryAttributethat supportCalleeTypeandConditionMemberNames, matching the pattern of the existingConditionalFact/ConditionalTheoryattributes.These derive from
CulturedFactAttribute/CulturedTheoryAttribute(xunit v3 only) and use the existingConditionalTestDiscoverer.EvaluateSkipConditionsto setSkipat construction time.Microsoft.DotNet.XUnitV3Extensions/src/:CulturedConditionalFactAttribute— extendsCulturedFactAttributeCulturedConditionalTheoryAttribute— extendsCulturedTheoryAttributeCulturedConditionalAttributeTests.cs: skip state validation, property exposure (CalleeType,ConditionMemberNames,Cultures), theory data passthrough, culture verification viaCultureInfo.CurrentCultureTo double check: