ionide / ionide-vscode-fsharp

VS Code plugin for F# development
http://ionide.io
MIT License
850 stars 276 forks source link

Getting errors when trying to execute unit tests with special characters in the names via the test explorer #1923

Closed Numpsy closed 9 months ago

Numpsy commented 10 months ago

Describe the bug

I have a test project which uses Expecto and is using testTheory to create tests. The generated test cases use the 'theory' data in the name, which resulted in some test cases whose name contains a =.

The test explorer is displaying these test cases ok:

image

If I use the test explorer to run the whole test set, or just run the 'CLASSIFICATION' test on its own then the tests run ok. However, if I try to run any of the tests whose name contains an = then I get this error in the test output

An exception occurred while invoking executor 'executor://yolodev/expecto': Incorrect format for TestCaseFilter Error: Invalid Condition 'FullyQualifiedName=TitusMetadataParser Tests.parse a user metadata string with no value.CLASSIFICATION='. Specify the correct format and try again. Note that the incorrect format can lead to no test getting executed.

I then tried running the xUnit based tests from Avalonia.FuncUI and found that there are similar issues with test cases that have '(' or ')' in the name, for example running

https://github.com/fsprojects/Avalonia.FuncUI/blob/ffc04eb5f1f4865f48b1d0bac3c9cd9bb4bed33a/src/Avalonia.FuncUI.UnitTests/VirtualDom/VirtualDom.ModuleTests.fs#L151C19-L151C19

image

got me this error

[xUnit.net 00:00:00.47] Avalonia.FuncUI.UnitTests: Exception filtering tests: Incorrect format for TestCaseFilter Missing Operator '|' or '&'. Specify the correct format and try again. Note that the incorrect format can lead to no test getting executed.

No test matches the given testcase filter `(FullyQualifiedName=Avalonia.FuncUI.UnitTests.VirtualDom.ModuleTests.integration test 'VirtualDom.updateRoot' (from null to Button to TextBlock))` in s:\GitHubForks\MyFuncUI\src\Avalonia.FuncUI.UnitTests\bin\Debug\net6.0\Avalonia.FuncUI.UnitTests.dll

Fwiw, the text explorer in Visual Studio will run both of these cases.

Steps to reproduce

Try to use the test explorer to execute a single test whose name contains an = or (/)

Link to sample reproduction

The tests in the set at https://github.com/fsprojects/Avalonia.FuncUI/blob/ffc04eb5f1f4865f48b1d0bac3c9cd9bb4bed33a/src/Avalonia.FuncUI.UnitTests/VirtualDom/VirtualDom.ModuleTests.fs#L151C19-L151C19 can reproduce the issue.

Expected behaviour

The selected tests should pass.

Machine info

TheAngryByrd commented 10 months ago

cc @farlee2121

Numpsy commented 10 months ago

Taking a wild stab at it, I wonder if it the filter strings need escaping somewhere around

https://github.com/ionide/ionide-vscode-fsharp/blob/63a98d46813a971a1c5e2f6d8960374fa1546f07/src/Components/TestExplorer.fs#L996C17-L996C17

?

farlee2121 commented 10 months ago

I looked into escaping some when investigating the errors with NUnit and test names containing spaces.

There is a little documentation about escaping in filter expressions.

Numpsy commented 10 months ago

I found this bit when I was looking up info on the error: https://github.com/microsoft/vstest-docs/blob/main/docs/filter.md

Numpsy commented 9 months ago

I had a go with a local debug build of Ionide using the change at https://github.com/Numpsy/ionide-vscode-fsharp/commit/272db3c607b491e11780e319586272901eae140b (using the same approach as the vstest escaping code at https://github.com/microsoft/vstest/blob/main/src/Microsoft.TestPlatform.ObjectModel/Utilities/FilterHelper.cs) and that did seem to fix the two errors I was getting (I haven't tried it on anything besides those).

Would that be the right sort of approach to be taking?

farlee2121 commented 9 months ago

If it works for vstest, then it'll probably work for us too.

I don't see any apparent issues. It covers all the documented special characters, escapes the test name only (not chars intended as expression operators), and looks like it'd scale for long strings (i.e. a bunch of individually selected tests to run).

Numpsy commented 9 months ago

My issue looks fixed in 7.12.2, so I'll close this now.