Skip to content
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.

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

Xunit theory filter #1943

Merged
merged 3 commits into from
Oct 6, 2023
Merged

Conversation

farlee2121
Copy link
Contributor

WHAT

#1935

#1936 got parameterized tests running as part of larger groups.
This PR continues that work and allows individual cases to be run individually from
the test explorer (i.e. clicking the run button on a theory case in the test explorer)

Note that for MSTest, it will actually run all of the theory cases because there doesn't appear to be a way to filter to a single parameterized test case in MSTest. It'll still look right, but multiple cases will be run if they try to debug.
All other frameworks (xUnit, nUnit, Expecto) can run individual parameterized cases and will debug as expected.

HOW

Mostly through modifying how we build the dotnet test filter expressions based on which test framework the project uses.

Nesting Deferred

I decided not to tackle nesting parameterized cases under a dedicated parent item in the test explorer (like Visual Studio does).
I'm still contemplating the options. Breaking the assumption that the explorer hierarchy matches the test name hierarchy could cause a lot of problems and complexity.

Doesn't appear to be possible to filter to only one
case for parameterized tests in MSTest.
Instead, we truncate the filter string to run all the cases for that method.
This looks close enough to the real thing unless you debug.
@@ -270,6 +280,10 @@ module TrxParser =
Some TestFrameworkId.NUnit
else if String.startWith "executor://mstest" adapterTypeName then
Some TestFrameworkId.MsTest
else if String.startWith "executor://xunit" adapterTypeName then
Some TestFrameworkId.XUnit
else if String.startWith "executor://yolodev" adapterTypeName then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now need to identify NUnit, xUnit, MSTest to deal with framework quirks.
Expecto was the only one left out, so I figured it's more intuitive if we match for it too.

@@ -1067,7 +1081,7 @@ module TestDiscovery =
trxTestsPerProject
|> Array.map (fun (project, trxDefs) ->
let projectPath = ProjectPath.ofString project.Project
let heirarchy = TrxParser.inferHierarchy trxDefs
let hierarchy = TrxParser.inferHierarchy trxDefs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a spelling fix

@@ -1180,17 +1193,32 @@ module Interactions =

let testToFilterExpression (test: TestItem) =
let fullTestName = TestItem.getFullName test.id
let fullName = escapeFilterExpression fullTestName
let escapedTestName = escapeFilterExpression fullTestName
Copy link
Contributor Author

@farlee2121 farlee2121 Oct 5, 2023

Choose a reason for hiding this comment

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

just a rename to indicate the intent of how the test name changed

@@ -1412,10 +1439,18 @@ module Interactions =
|> Option.map (fun p -> p.kind = TestRunProfileKind.Debug)
|> Option.defaultValue false

let hasIncludeFilter =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually a small unrelated fix.
Project-level items were constructing test filter clauses for all their top-level children.
But, that meant it wouldn't discover top-level tests if they weren't in the explorer yet.

The new behavior is to use no filter if just the top-level project is being run.
I can split this out if you'd prefer

$"(FullyQualifiedName~{testPart})"
else if test.TestFramework = TestFrameworkId.XUnit then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the core of the change. It customizes the filter expression based on test framework.

@@ -1544,7 +1579,12 @@ module Interactions =
logger.Error(message, buildFailures |> List.map ProjectPath.fromProject)

else
let librariesCapableOfListOnlyDiscovery = set [ "Expecto"; "xunit.abstractions" ]
let detectablePackageToFramework =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

xUnit and expecto can be discovered with --list-tests so we need to set the test framework in this discovery method too

Copy link
Member

@TheAngryByrd TheAngryByrd left a comment

Choose a reason for hiding this comment

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

🎸 Thank you so much for your continued work on this!

@TheAngryByrd TheAngryByrd merged commit 81cb6b5 into ionide:main Oct 6, 2023
2 checks passed
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.

2 participants