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

Include module information on TestAdapterEntry #1071

Merged
merged 9 commits into from
Mar 11, 2023

Conversation

kojo12228
Copy link
Contributor

Add a new field TestAdapterEntry.ModuleType, to show whether the given entry is a module, and separately a module with a suffix (in IL). The allows for handling the following two cases which previously couldn't be handled by the existing record:

namespace TestNameSpace

module Outer =
    [<TestFixture>]
    module Inner =
        [<Test>]
        let Test1 () =
            Assert.Pass ()

        module Innermost =
            [<Test>]
            let Test2 () =
                Assert.Pass()

module Outer2 =
    [<TestFixture>]
    type InnerClass() =
        [<Test>]
        member this.Test1 () =
            Assert.Pass()

type SharedName() =
    do ()

[<TestFixture>]
module SharedName =
    [<Test>]
    let Test1 () =
        Assert.Pass()
Test Class Name FullName in Code
TestNameSpace.Outer+Inner+Innermost.Test2 TestNameSpace.Outer.Inner.Innermost.Test2
TestNameSpace.SharedNameModule.Test1 TestNameSpace.SharedName

See ionide/ionide-vscode-fsharp#1756 for more context.

@baronfel
Copy link
Contributor

This approach looks great - I've started the tests and I'm happy to merge once green. Would you be willing to continue updating the test explorer in Ionide to match once this is released?

@baronfel
Copy link
Contributor

Before I merge, if you wanted to get a PR prepped on the Ionide side you could build this branch, get the path to the newly-created fsautocomplete.dll, and then set that value as the FSharp.fsac.netcoreDllPath setting in Ionide to cause Ionide to load your local FSAC instance.

@kojo12228
Copy link
Contributor Author

The failing Build on windows-latest for 7.0 preview is odd given it seems Build on windows-latest for repo global.json should be running the same thing. Would you advise I simplify the test, or split the XUnit and NUnit tests into multiple tests?

I'll take a look at creating a Ionide PR. Turns out it's more complicated than I thought, since the part of the code that tries to construct the full name has no access to the TestAdapterEntry and TestItem from VSCode has no field which I could append this information to 🤔.

@kojo12228
Copy link
Contributor Author

Turns out I missed a case:

module OtherSpace

module Outer2 =
    [<TestFixture>]
    type InnerClass() =
        [<Test>]
        member this.Test1 () =
            Assert.Pass()

Leads to OtherSpace+Outer2+InnerClass which thankfully I had included in my project but didn't remember to include in this PR.

@nojaf
Copy link
Contributor

nojaf commented Mar 11, 2023

Great work @kojo12228!

@baronfel baronfel merged commit d025a2e into ionide:main Mar 11, 2023
@baronfel
Copy link
Contributor

yes, very well done 🎆

@kojo12228
Copy link
Contributor Author

Thanks @baronfel! And thanks @nojaf, our chat was very helpful, I decided to plough through the change that night. https://fsprojects.github.io/fantomas/reference/index.html was very useful, thought search was a bit difficult with the really long DU cases!

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.

3 participants