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

exception occurred while invoking executor Unexpected token '(' at position 100 in selection expression. #1510

Closed
livebe01 opened this issue Nov 6, 2024 · 17 comments · Fixed by #1519
Assignees
Labels

Comments

@livebe01
Copy link

livebe01 commented Nov 6, 2024

I have the following test

[Test]
[TestCase(ThisEnum.Option1, "ABC")]
[TestCase(ThisEnum.Option2, "DEF")]
[TestCase(ThisEnum.Option3, "GHI")]
public void TestSomething(ThisEnum thisEnum, string expected)
{
// do something
}

I can run the tests individually, and as a group in Rider w/o issue. But, for some use cases, it'd be really useful to be able to run parameterized tests individually, from the CLI.

I'm developing on a Mac using Rider. My project is using "NUnit" Version="3.13.3" and "NUnit3TestAdapter" Version="4.3.1".

I run the following from the Mac terminal:
dotnet test -- NUnit.Where="test==Some.Name.Space.SomeClass.TestSomething(EnumValue,\"ABC\")"

and get this error:

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
An exception occurred while invoking executor 'executor://nunit3testexecutor/': Unexpected token '(' at position 100 in selection expression.
Stack trace:
   at NUnit.Engine.TestSelectionParser.Expect(Token[] valid)
   at NUnit.Engine.TestSelectionParser.Parse(String input)
   at NUnit.Engine.TestFilterBuilder.GetFilter()
   at NUnit.VisualStudio.TestAdapter.NUnitTestFilterBuilder.FilterByWhere(String where) in C:\repos\nunit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnitTestFilterBuilder.cs:line 93
   at NUnit.VisualStudio.TestAdapter.NUnit3TestExecutor.RunTests(IEnumerable`1 sources, IRunContext runContext, IFrameworkHandle frameworkHandle) in C:\repos\nunit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnit3TestExecutor.cs:line 129
   at Microsoft.VisualStudio.TestPlatform.Common.ExtensionDecorators.SerialTestRunDecorator.RunTests(IEnumerable`1 sources, IRunContext runContext, IFrameworkHandle frameworkHandle)
   at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Execution.RunTestsWithSources.InvokeExecutor(LazyExtension`2 executor, Tuple`2 executorUriExtensionTuple, RunContext runContext, IFrameworkHandle frameworkHandle)
   at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Execution.BaseRunTests.RunTestInternalWithExecutors(IEnumerable`1 executorUriExtensionMap, Int64 totalTests)

When I run dotnet test -- NUnit.Where="test==Some.Name.Space.SomeClass.TestSomething", all three tests run w/o error. I wonder if you can point me at what I might be doing wrong or what I might try to get passed this.

Thanks!

@stevenaw
Copy link
Member

stevenaw commented Nov 6, 2024

Hi @livebe01
I don't have a Mac myself to test this, but can you try escaping the ( and ) by putting a back slash in front? For example:

dotnet test -- NUnit.Where="test==Some.Name.Space.SomeClass.TestSomething\(EnumValue,\"ABC\"\)"

@livebe01
Copy link
Author

livebe01 commented Nov 7, 2024

Thanks @stevenaw, I tried your suggestion, but it results in the same error/behavior.

It's strange to see C:\repos\nunit\nunit3-vs-adapter\src referenced in the trace. That's a Windows path.

@OsirisTerje
Copy link
Member

The same error happens on Windows. It should work according to the docs https://docs.nunit.org/articles/nunit/running-tests/Test-Selection-Language.html#simple-expressions

The error happens in the NUnit.Engine, so I'll transfer the issue there.
Repro code here: https://github.com/nunit/nunit.issues/tree/main/Issue4880

@OsirisTerje OsirisTerje transferred this issue from nunit/nunit Nov 7, 2024
@mikkelbu
Copy link
Member

mikkelbu commented Nov 8, 2024

I took a quick look at this when it was reported and as far as I quickly could determine then

private static readonly Token LPAREN = new Token(TokenKind.Symbol, "(");
is only used for expressions inside parenthesis and not for arguments (and this is somewhat confirmed by looking at the tokenizer - https://github.com/nunit/nunit-console/blob/d238d33023c1ffe96b2c6bf44f2077569c17e763/src/NUnitEngine/nunit.engine/Services/Tokenizer.cs#L86C30-L86C47 ).

I'm actually unsure if this have ever worked (perhaps the documentation and code is out of sync?).

Ps. NUnitLite seems to have the same problem

@CharliePoole
Copy link
Member

CharliePoole commented Nov 8, 2024

If this is an error in the engine, we should be able to repro it with a failing unit test for the tokenizer and/or the parser. I'll try to do that when I get back to work in a few weeks but if somebody wants to start on it now, please go ahead. We haven't often checked in failing tests deliberately, but that's actually a good way to prove a bug and does no harm since we won't merge a failing branch.

It's harder for us if the failure only occurs on a Mac, but @livebe01 could give it a try as well. We run into this problem (lack of devs with Macs on the team) periodically.

@livebe01
Copy link
Author

livebe01 commented Nov 8, 2024

I took a quick look at this when it was reported and as far as I quickly could determine then

private static readonly Token LPAREN = new Token(TokenKind.Symbol, "(");

is only used for expressions inside parenthesis and not for arguments (and this is somewhat confirmed by looking at the tokenizer - https://github.com/nunit/nunit-console/blob/d238d33023c1ffe96b2c6bf44f2077569c17e763/src/NUnitEngine/nunit.engine/Services/Tokenizer.cs#L86C30-L86C47 ).
I'm actually unsure if this have ever worked (perhaps the documentation and code is out of sync?).

Ps. NUnitLite seems to have the same problem

Per nunit/docs#316, it looks like the feature may have worked at some earlier point.

@livebe01
Copy link
Author

livebe01 commented Nov 8, 2024

It's harder for us if the failure only occurs on a Mac, but @livebe01 could give it a try as well. We run into this problem (lack of devs with Macs on the team) periodically.

For sure. Let me know if there's something you'd like me to try.

@CharliePoole
Copy link
Member

If you are able to build a test case that excercises the engine at a lower level, that would help. See our Tokenizer tests and TestSelectionParser tests for examples. This might be simpler for you than building the entire engine and runner, although that's an option as well.

Have you looked at the console output yet? It may actually show how the runner is interpreting your where filter.

@OsirisTerje
Copy link
Member

It's harder for us if the failure only occurs on a Mac,

I tested it on windows, same results.
And I tested it using dotnet test which invokes the engine.

@livebe01
Copy link
Author

livebe01 commented Nov 8, 2024

It's harder for us if the failure only occurs on a Mac,

I tested it on windows, same results. And I tested it using dotnet test which invokes the engine.

I wanted to add, I get the same error whether running dotnet test or nunit3-console if that provides any additional information...

@CharliePoole
Copy link
Member

It's harder for us if the failure only occurs on a Mac,

I tested it on windows, same results. And I tested it using dotnet test which invokes the engine.

OK, I missed that. I labeled it a bug and if nobody gets to it before the 25th, I'll work it.

@stevenaw
Copy link
Member

stevenaw commented Nov 9, 2024

I've been able to try a few things tonight here.
I was able to avoid the parser error by enclosing the test name in single quotes:

dotnet test -- NUnit.Where="test=='Some.Name.Space.SomeClass.TestSomething(EnumValue,\"ABC\")'"

However there still seems to be an issue with the test selection. I can correctly select a test method which expects an int or an enum but not a string. It does appear there is an existing test for exactly that scenario, though, which passes on the engine side.

I have to stop the investigation for now, but if the engine test proves correct XML generation then I am wondering if perhaps this is a framework issue.

dotnet test -- NUnit.Where="test=='Issue1510.Tests.TestSomethingEnum(Option1)'"
dotnet test -- NUnit.Where="test=='Issue1510.Tests.TestSomethingInt(42)'"
dotnet test -- NUnit.Where="test=='Issue1510.Tests.TestSomethingString(\"ABC\")'"

[TestCase(42)]
public void TestSomethingInt(int expected)
{
}

[TestCase(ThisEnum.Option1)]
public void TestSomethingEnum(ThisEnum expected)
{
}

    [TestCase("ABC")]
    public void TestSomethingString(string expected)
    {
        // this one can't be run
    }

@stevenaw
Copy link
Member

stevenaw commented Nov 9, 2024

Free time is a little unpredictable at the moment for me so I've put up a PR with some of the tests I'd written as part of my investigation tonight: #1513

@CharliePoole
Copy link
Member

I've experimented with @stevenaw 's branch and it's tests. They all seem to pass whether I use single quotes or not.

So, the tests are verifying that the engine accepts an argument as input and creates a filter containing valid XML that exactly matches our format for filters. Two important things are not demonstrated by the tests:

  1. That the same string entered on the command-line actually reaches the engine without change. This will vary for each OS and requires manual testing. Our test cases are written conservatively using a format that we expect to work on Windows, Linux and Mac. That's why, for example we always enclose a test name in single quotes, whether it needs it or not.

  2. That the format of a filter we generate in the engine is exactly the same as what the engine expects. It's possible to add automated tests for this but it's not done currently.

@mikkelbu Indicated the problem also exists in NUnitLite, so I think I'll experiment with that first.

@CharliePoole
Copy link
Member

Let's get NUnitLite out of the way first...

It has the same problem purely because it contains copies of TestSelectionParser and Tokenizer.cs that I made the copies a long time ago. Maybe we can remove that duplication at some point, but it doesn't seem a high priority.

@CharliePoole
Copy link
Member

Working with the repro that @OsirisTerje provided, I can make it work using the following command-line...

dotnet test -- NUnit.Where="test=='Issue4880.Tests.Test1(42,\"abc\")'" <= single quote followed by double quote

That's pretty obscure and clearly unsatisfactory. As far as I can tell, dotnet test removes the outer quotes and the adapter passes test=='Issue4880.Tests.Test1(42,\"abc\")' to the engine as a where clause. @OsirisTerje can you confirm that? Maybe we can somehow work to improve that syntax.

Using the console command-line, we want the engine to receive the same where clause. Under windows, all of the following do that and run the single test correctly...

nunit3-console Issue4880.dll --where=test=='Issue4880.Tests.Test1(42,\"abc\")'
nunit3-console Issue4880.dll --where: test=='Issue4880.Tests.Test1(42,\"abc\")'
nunit3-console Issue4880.dll --where test=='Issue4880.Tests.Test1(42,\"abc\")'

Note that there is a small change in all of these examples, as compared to what was given originally. I have eliminated the blank space after the comma, so as to provide the exact test name, which is Issue4880.Tests.Test1(42,"abc") and not Issue4880.Tests.Test1(42, "abc")!

So where does that leave us?

WRT the console runner and engine, I think it would be reasonable to ignore white space within the parentheses that surround the arguments, but not within any individual string argument. I'll look at that for this issue. In addition, I think we need to verify that the docs emphasize sufficiently that escape requirements for the command-line vary according to the operating system. The engine itself also requires escaping certain things and that too should be documented.

WRT using the adapter via dotnet test my opinion is that the required syntax, using single and double quotes is unacceptable. If folks agree, there should probably be a separate issue for it.

WRT the framework, I'm not seeing any problem so long as the name of the test is specified correctly.

@CharliePoole
Copy link
Member

Curiouser and curiouser. :-)

It is possible to make the command line work with double quotes on Windows. You "merely" have to enter...

--where test==\"Issue4880.Tests.Test1(42,\\\"abc\\\")\"

This seems to be due to the byzantine nature of Windows command-line processing wrt double quotes. This partial output from cmd /?, although specific to the /C '/K` switches, gives a hint.

If /C or /K is specified, then the remainder of the command line after
the switch is processed as a command line, where the following logic is
used to process quote (") characters:

    1.  If all of the following conditions are met, then quote characters
        on the command line are preserved:

        - no /S switch
        - exactly two quote characters
        - no special characters between the two quote characters,
          where special is one of: &<>()@^|
        - there are one or more whitespace characters between the
          two quote characters
        - the string between the two quote characters is the name
          of an executable file.

    2.  Otherwise, old behavior is to see if the first character is
        a quote character and if so, strip the leading character and
        remove the last quote character on the command line, preserving
        any text after the last quote character.

and, further along, ...

The special characters that require quotes are:
     <space>
     &()[]{}^=;!'+,`~

For the sort of processing we're doing here, we would probably be better advised to parse the command-line text directly, rather than relying on the shell to split out arguments for us. Unfortunately, that's not how the command-line processing is designed. At the time the console was originally written (around 2000) the shell did much less processing.

Anyway, it's very clear that the command-line handling in windows itself is causing most of the problem. The immediate workaround until this issue is closed is to...

  1. Avoid use of double quotes around the entire where clause. Prefer single quotes.
  2. Use escaped double quotes around any string arguments specified
  3. Ensure that you use the exact name of any test and don't add extra spaces. Use --explore to get a list of all test case names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants