Skip to content

Support xunit.v3 in Microsoft.DotNet.XUnitExtensions#15668

Merged
akoeplinger merged 7 commits intomainfrom
dev/ygerges/xunit.v3
Mar 28, 2025
Merged

Support xunit.v3 in Microsoft.DotNet.XUnitExtensions#15668
akoeplinger merged 7 commits intomainfrom
dev/ygerges/xunit.v3

Conversation

@Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Mar 26, 2025 •
edited
Loading

Part of #15654

There are some types that used to be present in Microsoft.DotNet.XUnitExtensions but no longer present in Microsoft.DotNet.XUnit3Extensions:

  1. ConditionalFactAttribute and ConditionalTheoryAttribute:
    • If using the parameterless constructor of the attribute, use FactAttribute and TheoryAttribute instead, and replace throw new SkipTestException(...) with Assert.Skip(...).
    • @ViktorHofer Do you know for the non-parameterless constructor? Is there a native replacement in xunit.v3 for that? Or maybe we should keep the attributes for this case? 😕
  2. ParallelTheoryAttribute: No replacement for that currently. I can't find any usages of it with grep.app. If requested by someone, we can work on adding it.

Manually tested scenarios:

ConditionalClassAttribute

namespace TestForArcadev3;

[ConditionalClass(typeof(UnitTest1), nameof(MyCondition))]
public class UnitTest1
{
    public static bool MyCondition => true;

    [Fact]
    public void Test1() => Assert.Fail("UnitTest1");
}

[ConditionalClass(typeof(UnitTest2), nameof(MyCondition))]
public class UnitTest2
{
    public static bool MyCondition => false;

    [Fact]
    public void Test1() => Assert.Fail("UnitTest2");
}

Running with --filter-query /[category!=failing]:

  1. UnitTest1.Test1 is run and fails as expected.
  2. UnitTest2.Test1 isn't run.

DotNetOnlyFact

namespace TestForArcadev3;

public class UnitTest1
{
    [DotNetOnlyFact]
    public void Test1() => Assert.Fail("UnitTest1");
}
  1. dotnet test -f net8.0: Test is run and fails.
  2. dotnet test -f net472: Test isn't run and executable finishes with exit code 8 (zero tests ran)

Youssef1313
Youssef1313 commented Mar 26, 2025
View reviewed changes
@Youssef1313 Youssef1313 mentioned this pull request Mar 26, 2025
Open
4 tasks
@Youssef1313 Youssef1313 requested a review from ViktorHofer March 26, 2025 12:23
@akoeplinger
Copy link
Member

akoeplinger commented Mar 27, 2025

Do you know for the non-parameterless constructor? Is there a native replacement in xunit.v3 for that? Or maybe we should keep the attributes for this case? 😕

yeah I think we should keep it. that said, now that I look at it I wonder why ConditionalFact/Theory don't use the same approach as https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.XUnitExtensions/src/Attributes/LinuxOnlyFactAttribute.cs where we set the Skip property in the ctor instead of the current complicated Discoverer + failing trait. Was that not available in earlier xunit versions?

@Youssef1313
Copy link
Member Author

Youssef1313 commented Mar 27, 2025

@akoeplinger I don't know about the history. But there is a fundamental difference between both approaches. Using Skip will cause the test to always be skipped when the condition isn't met (and the test will be reported in test results). However, using failing trait depends on whether or not you filter by category!=failing:

  1. If category!=failing filter is present, I think the test won't run and will also not be reported in test results at all.
  2. If the filter isn't present, the test will be run regardless.

I'd tend to avoid making fundamental/behavioral changes as part of a simple migration to xunit.v3.

@akoeplinger
Copy link
Member

akoeplinger commented Mar 27, 2025

Yeah. It's just that we've been thinking of removing the need for the failing trait since it's confusing: #15195

I agree we can tackle that separately.

@Youssef1313
Copy link
Member Author

Youssef1313 commented Mar 27, 2025

@akoeplinger Thanks! Is this good to merge soon then (probably with a second review)?

@akoeplinger akoeplinger merged commit 4bb7855 into main Mar 28, 2025
11 checks passed
@akoeplinger akoeplinger deleted the dev/ygerges/xunit.v3 branch March 28, 2025 11:25
This was referenced Mar 28, 2025
Draft
Merged
@jkotas jkotas mentioned this pull request Oct 3, 2025
Merged
Copilot AI mentioned this pull request Feb 6, 2026
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akoeplinger akoeplinger akoeplinger approved these changes

@ViktorHofer ViktorHofer ViktorHofer approved these changes

@agocke agocke Awaiting requested review from agocke

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Youssef1313 @akoeplinger @ViktorHofer