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

Nunit 4.0.x Test case selection is incorrect with certain test data #4584

Open
johnjaylward opened this issue Dec 7, 2023 · 13 comments
Open
Labels
awaiting:team Investigate We will look into this

Comments

@johnjaylward
Copy link

We have certain test cases that are using test data and expected results.

When the tests run in Microsoft Visual Studio 2022 (64-bit) Version 17.8.2, the tests get ignored.

I've created a sample project that recreates the issue: https://github.com/johnjaylward/Nunit4_TestcaseSelectionIssue

I managed to find a work around that does let the tests run by adding another Test with the same name, but does not use parameters.

In the test project the SelectionFail tests show the bug while the SelectionSuccess test shows the workaround.

image

@johnjaylward
Copy link
Author

johnjaylward commented Dec 7, 2023

Additional information...

When run from the command line using dotnet test, I get the following output (which I believe is expected):

** Edit. I updated the "SelectionFail" tests to pass if they actually are run, so I updated the command line output below.

Microsoft (R) Test Execution Command Line Tool Version 17.8.0 (x64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
  Skipped TestSuccess [< 1 ms]

Passed!  - Failed:     0, Passed:     8, Skipped:     1, Total:     9, Duration: 21 ms - Nunit4_TestcaseSelectionIssue.dll (net8.0)

@johnjaylward
Copy link
Author

I also want to note that this was not an issue with Nunit 3.14.0

@OsirisTerje OsirisTerje added the Investigate We will look into this label Dec 7, 2023
@OsirisTerje
Copy link
Member

OsirisTerje commented Dec 7, 2023

I tried your code, and I do see the same in Visual Studio.
When I add a runsettings file, and dump the content from the framework/engine, I also see that no tests from SelectionFail appears. They are sent down from the explorer/adapter, but doesnt come back.

It is something with the naming here, if I do a SetName on each data to a "normal" name, then it works.

image

I also see that it works from the command line.
Looking at the dump files, the dotnet test doesn't use any filter, as expected, in this case

<TestFilter>
   <filter/>
</TestFilter>

Using Test Explorer, it does send all down as a list of testcases, so we then get a rather ugly filter:
(just start of shown)

  <filter><or><test>Nunit4_TestcaseSelectionIssue.SelectionFail.TestFailure([
  1,
  2,
  3,
  4,
  5,
  6
],4)</test><test>Nunit4_TestcaseSelectionIssue.SelectionFail.TestFailure([
  1,
  2,
  3,
  4,
  5,
  6
],2)</test><test>Nunit4_TestcaseSelectionIssue.SelectionFail.TestFailure([
  1,

There is then something in the framework/engine that doesn't like these test names. Why an extra test should make them appear again beats me right now.

As a workaround: You can force it to not use any filter if you add <AssemblySelectLimit>2</AssemblySelectLimit> (with any number less than the number of tests you have) to your .runsettings file. Then it will always run all tests.

@manfred-brands @stevenaw Any of you have any ideas where the selection for these filters are being done, or what can cause this behaviour?
As mentioned I tried with a .SetName("Test1") and upwards, and then all work nice, so it has to do with the resulting naming this kind of data give.

@johnjaylward
Copy link
Author

I'm not familiar with the .runsettings file. However, setting up my file like the below screenshot appears to still cause some items to be skipped, but this time it's the "successes" that got skipped and not the "fails"

image

@johnjaylward
Copy link
Author

ok, instead of using the default .runsettings as provided by MS, I used the one in this repo, and I got similar results to what you showed:

image

@johnjaylward
Copy link
Author

What are the implications of setting the AssemblySelectLimit value? Is it just for the test case filtering, or does it alter the test runs in another way?

@OsirisTerje
Copy link
Member

It doesn't affect anything but the filtering of test cases sent to the framework. That setting is meant to fix the issue that Test Explorer was changed at some time to not send us Test All anymore, but just a list of testcases. If you have a lot of test cases, say >2000, it would slow down the test simply because of the parsing of that long list of tests. So when the number of tests are above this limit, we just clean the filter and let all tests run. If you define a category filter, it will still honor that filter.

@stevenaw
Copy link
Member

@OsirisTerje sorry, I had missed that you had pinged for thoughts here. I'm not at a computer for a few days, but will try to look soon. Definitely need to step through it to understand it.

Initial thoughts: I can't recall any changes in how we'd enumerate something like a jsonarray to generate test cases but I think the biggest change in that area this release was async enumeration. I'm unsure if jsonarray could be taking the async path there?

@stevenaw
Copy link
Member

Oh, reading further I see that perhaps this is on the filtering side, as you mention.

@mikkelbu
Copy link
Member

mikkelbu commented Feb 4, 2024

I cannot think of any changes that should affect this, so I'll have to try it out to come with a better answer

@stevenaw
Copy link
Member

I've been able to experiment a bit with the repro.

It seemed as though commenting out the ignore didn't change anything, though the presence of another test did seem to be required to get the parameterized "success"case to run.

@stevenaw
Copy link
Member

I'm beginning to wonder if this is a line endings issue within the test name.

I see \r\n here in DefaultTestCaseBuilder.BuildFrom
image

And \n here in the passed in filter in NUnitTestAssemblyRunner.Run()
image

The filter itself is coming in as \r\n. It seems that the parsing of the <test> element yields a TNode.Value with \n while TNode.OuterXml itself instead shows \r\n.

At a glance there were some changes to OrFilter and FullNameFilter as well as InFilter.TryOptimize. It does look like it's trying to use optimize the filter into an InFilter. I'm wondering if the hash-based checking of the InFilter is what's conflicting with potentially mixed line endings.

I have to stop investigation for the night but hopefully these intermediate findings may spark a team member's memory.

@stevenaw
Copy link
Member

This doesn't appear related to the in filter optimization as commenting it out doesn't seem to change anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting:team Investigate We will look into this
Projects
None yet
Development

No branches or pull requests

4 participants