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
Changes from all commits
Commits
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
74 changes: 63 additions & 11 deletions src/Components/TestExplorer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ module ListExt =
let mapPartitioned f (left, right) =
(left |> List.map f), (right |> List.map f)

module Dict =
let tryGet (d: Collections.Generic.IDictionary<'key, 'value>) (key) : 'value option =
if d.ContainsKey(key) then Some d[key] else None

module CancellationToken =
let mergeTokens (tokens: CancellationToken list) =
let tokenSource = vscode.CancellationTokenSource.Create()
Expand Down Expand Up @@ -227,6 +231,12 @@ module TestFrameworkId =
[<Literal>]
let MsTest = "MSTest"

[<Literal>]
let XUnit = "XUnit"

[<Literal>]
let Expecto = "Expecto"

type TestResult =
{ FullTestName: string
Outcome: TestResultOutcome
Expand Down Expand Up @@ -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.

Some TestFrameworkId.Expecto
else
None

Expand Down Expand Up @@ -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


let fromTrxDef (hierarchy: TestName.NameHierarchy<TrxParser.UnitTest>) =
// NOTE: A project could have multiple test frameworks, but we only track NUnit for now to work around a defect
Expand All @@ -1088,11 +1102,10 @@ module TestDiscovery =

TestItem.fromNamedHierarchy testItemFactory tryGetLocation projectPath hierarchy

let projectTests = heirarchy |> Array.map fromTrxDef
let projectTests = hierarchy |> Array.map fromTrxDef

TestItem.fromProject testItemFactory projectPath project.Info.TargetFramework projectTests)


treeItems

module Interactions =
Expand Down Expand Up @@ -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


if fullName.Contains(" ") && test.TestFramework = TestFrameworkId.NUnit then
if escapedTestName.Contains(" ") && test.TestFramework = TestFrameworkId.NUnit then
// workaround for https://github.com/nunit/nunit3-vs-adapter/issues/876
// Potentially we are going to run multiple tests that match this filter
let testPart = fullName.Split(' ').[0]
let testPart = escapedTestName.Split(' ').[0]
$"(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.

// NOTE: using DisplayName allows single theory cases to be run for xUnit
let operator = if test.children.size = 0 then "=" else "~"
$"(DisplayName{operator}{escapedTestName})"
else if test.TestFramework = TestFrameworkId.MsTest && String.endWith ")" fullTestName then
// NOTE: MSTest can't filter to parameterized test cases
// Truncating before the case parameters will run all the theory cases
// example parameterized test name -> `MsTestTests.TestClass.theoryTest (2,3,5)`
let truncateOnLast (separator: string) (toSplit: string) =
match toSplit.LastIndexOf(separator) with
| -1 -> toSplit
| index -> toSplit.Substring(0, index)

let truncatedTestName = truncateOnLast @" \(" escapedTestName
$"(FullyQualifiedName~{truncatedTestName})"
else if test.children.size = 0 then
$"(FullyQualifiedName={fullName})"
$"(FullyQualifiedName={escapedTestName})"
else
$"(FullyQualifiedName~{fullName})"
$"(FullyQualifiedName~{escapedTestName})"

let filterExpression =
tests |> Array.map testToFilterExpression |> String.concat "|"
Expand Down Expand Up @@ -1282,7 +1310,6 @@ module Interactions =

tryFind "Expected:", tryFind "But was:"


{ FullTestName = trxResult.UnitTest.FullName
Outcome = !!trxResult.UnitTestResult.Outcome
ErrorMessage = trxResult.UnitTestResult.Output.ErrorInfo.Message
Expand Down Expand Up @@ -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

let isOnlyProjectSelected =
match tests with
| [| single |] -> single.id = (TestItem.constructProjectRootId project.Project)
| _ -> false

(Option.isSome runRequest.``include``) && (not isOnlyProjectSelected)

{ ProjectPath = projectPath
TargetFramework = project.Info.TargetFramework
ShouldDebug = shouldDebug
HasIncludeFilter = Option.isSome runRequest.``include``
HasIncludeFilter = hasIncludeFilter
Tests = replaceProjectRootIfPresent tests })

let runHandler
Expand Down Expand Up @@ -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

dict
[ "Expecto", TestFrameworkId.Expecto
"xunit.abstractions", TestFrameworkId.XUnit ]

let librariesCapableOfListOnlyDiscovery = set detectablePackageToFramework.Keys

let listDiscoveryProjects, trxDiscoveryProjects =
builtTestProjects
Expand All @@ -1559,6 +1599,17 @@ module Interactions =
let! testNames =
DotnetCli.listTests project.Project project.Info.TargetFramework false cancellationToken

let detectedTestFramework =
let getPackageName (pr: PackageReference) = pr.Name

project.PackageReferences
|> Array.tryPick (getPackageName >> Dict.tryGet detectablePackageToFramework)

let testItemFactory (testItemBuilder: TestItem.TestItemBuilder) =
testItemFactory
{ testItemBuilder with
testFramework = detectedTestFramework }

let testHierarchy =
testNames
|> Array.map (fun n -> {| FullName = n; Data = () |})
Expand Down Expand Up @@ -1602,6 +1653,7 @@ module Interactions =
let trxDiscoveredTests =
TestDiscovery.discoverFromTrx testItemFactory tryGetLocation makeTrxPath trxDiscoveryProjects


let listDiscoveredTests = listDiscoveredPerProject |> Array.map snd
let newTests = Array.concat [ listDiscoveredTests; trxDiscoveredTests ]

Expand Down