Testfx: MSTest.TestAdapter/TestFramework 2.1.0 broke existing F# tests with a period in their name.

[SomethingAwry]

F# methods/functions are allowed to have special characters in their names by book-ending the name with two back-ticks. (Used for making friendly test names, etc.)

namespace NS
open Microsoft.VisualStudio.TestTools.UnitTesting
[<TestClass>]
type TC () =
    [<TestMethod>]
    member __.``a.b`` () : unit =
        ()

It seems that prior to 2.1.0, only the test explorer was messed up, showing a 'group' called a containing a test b, but with 2.1.0 it's now broken them: the bogus b test fails: Method NS.TC.b does not exist. (although the Test Detail Summary still shows the correct name of a.b)

466 appears to have introduced a change that works by splitting on periods within testCase.FullyQualifiedName; perhaps that was the cause, though based on the existing test explorer behavior, perhaps it was something else?

[nohwnd]

I am sorry, that is very unfortunate that we did not catch this earlier. There was a lot of back and forth about how the name of the test should be figured out, and the splitting was one of the discussion points because at the last point only the whole name was available, but not the method name. So I am aware of what is causing this bug, I just don't know how to fix it.

Are you just reporting, or would you be willing to PR a fix for this?

[SomethingAwry]

Just reporting; I only hit it shortly before, and perused code more to confirm suspicions (undone a little as I confirmed the test explorer issue was prior).

[slang25]

I'm willing to take a look at this tonight and will report back

[nohwnd]

@slang25 it would be great if you added F# test project for this into acceptance tests (The project would go into testAssets folder) 🙂

[slang25]

Got me a failing test, the fix wasn't as trivial as I'd hope, we'll need to track some extra info. Will continue tomorrow.

[nohwnd]

@slang25 yeah I was working on this before, the problem there is that the original data are not available at that point and that why the original PR had the solution that we have now that is splitting the path on . which was reverted for a bit, and then causing the PR build to fail, because no test were matched, because the matching part had the full path to the test and not just the method name. But since in F# the method name is more flexible we are seeing this issue. So yes you are right. Hopefully this is not across public api that would need to change. If you need help with the PR feel free the push to your PR branch, it's a draft anyway, and then we can collab on the fix.

[nohwnd]

Fixed in #683