Skip to content

Skip methods in test-merging if they use the named argument 'Skip' for the Fact attributes #85495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 28, 2023
Merged

Skip methods in test-merging if they use the named argument 'Skip' for the Fact attributes #85495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45f7e6c
Skip methods in test-merging if they use the named argument 'Skip'
TIHan Apr 27, 2023
76611a0
Use ActiveIssueAttribute instead of FactAttribute with Skip. Add docu…
TIHan Apr 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/workflow/testing/libraries/filtering-tests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,15 @@ A common usage in the libraries tests is the following:
```

This is put on test classes to indicate that none of the tests in that class (which as usual run serially with respect to each other) may run concurrently with tests in another class. This is used for tests that use a lot of disk space or memory, or dominate all the cores, such that they are likely to disrupt any tests that run concurrently.

## FactAttribute and `Skip`

Another way to disable the test entirely is to use the `Skip` named argument that is used on the `FactAttribute`.

Example:
```cs
[Fact(Skip = "<reason for skipping>")]
```

If the reason for skipping is a link to an issue, it is recommended to use the `ActiveIssueAttribute` instead.
Otherwise, `Skip` allows for a more descriptive reason.
26 changes: 20 additions & 6 deletions src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,21 +464,35 @@ private static IEnumerable<ITestInfo> GetTestMethodInfosForMethod(IMethodSymbol
List<AttributeData> filterAttributes = new();
foreach (var attr in method.GetAttributesOnSelfAndContainingSymbols())
{
var hasSkip = attr.NamedArguments.Any(x => x.Key == "Skip");

switch (attr.AttributeClass?.ToDisplayString())
{
case "Xunit.ConditionalFactAttribute":
filterAttributes.Add(attr);
factAttribute = true;
if (!hasSkip)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a prime example for the newer C# switch syntax

case "Xunit.ConditionalFactAttribute" when !hasSkip:

though perhaps the right solution is to put all of these into filterAttributes so that filtering can then drop them like other filters do (and skip reporting, etc., would have a single place to be implemented).

{
filterAttributes.Add(attr);
factAttribute = true;
}
break;
case "Xunit.FactAttribute":
factAttribute = true;
if (!hasSkip)
{
factAttribute = true;
}
break;
case "Xunit.ConditionalTheoryAttribute":
filterAttributes.Add(attr);
theoryAttribute = true;
if (!hasSkip)
{
filterAttributes.Add(attr);
theoryAttribute = true;
}
break;
case "Xunit.TheoryAttribute":
theoryAttribute = true;
if (!hasSkip)
{
theoryAttribute = true;
}
break;
case "Xunit.ConditionalClassAttribute":
case "Xunit.SkipOnPlatformAttribute":
Expand Down
2 changes: 1 addition & 1 deletion src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public static int M8(byte arg0)
}
}

[Fact(Skip = "https://github.com/dotnet/runtime/issues/85081")]
[ActiveIssue("https://github.com/dotnet/runtime/issues/85081")]
public static int TestEntryPoint() {
var result = Test.Program.M8(1);
if (result != 255)
Expand Down