Issafalcon / neotest-dotnet

Neotest adapter for dotnet
MIT License
63 stars 20 forks source link

Specflow NUnit "no test matches the given testcase filter" #72

Closed teast closed 9 months ago

teast commented 9 months ago

Hi,

After the changes in 89db567e4dea2fab75c8cf0803a5ee4200660c0f (next commit after 1.2.0 label) the auto-generated specflow tests stop working.

The issue seems to be that the test class name gets duplicated in "FullyQualifiedName" search.

I have tried to pinpoint actual problem so I could come up with an solution but I was not able to do that.

My guess is that the treesitter query in DotnetNeotestAdapter.discover_positions is the one that double class name.

I have attached an small nunit test project where I have this issue.

Expected FullyQualifiedName: dummy_test.Features.DummyTestFeature.DummyScenario But it is: dummy_test.Features.DummyTestFeature.DummyTestFeature.DummyScenario

dummy_test.zip

teast commented 9 months ago

I did some more tests.

it seems that the problem is with the added treesitter query in nunit-queries.lua:

    ;; Matches test classes (no TestFixture attribute)
    (class_declaration
      name: (identifier) @namespace.name
    ) @namespace.definition

I created this small test class:

[TestFixture]
public class UnitTester
{
    [Test]
    public void DoTest()
    {
        Assert.IsTrue(true);
    }
}

If I remove [TestFixture] attribute on the class I will get the correct "fullyQualifiedName" but with it I get the same issue as mention before (class name is doubled)

I'm not sure if it is ok to just remove the treefilter above for nunit specific or if that would break something else?

Edit: Looks like I was too fast in commenting. I fetched the latest from git and it looks like for the normal unit test this works. but for SpecFlows it still have duplicate class names.

Could it be due to Specflow writing out the whole "NUnit.Framework.TextFixtureAttribute()" in the attribute?

Edit2:

With latest git version I was able to get it working by reverting following treesitter query in nunit-queries.lua (on line 36):

    ;; Matches test classes
    (class_declaration
      name: (identifier) @class.name
    ) @class.definition

With following:

    ;; Matches test classes
    (class_declaration
      (attribute_list
        (attribute
          name: (identifier) @attribute_name (#eq? @attribute_name "TestFixture")
        )
      )
      name: (identifier) @namespace.name
    ) @namespace.definition
Issafalcon commented 9 months ago

Thanks for raising the issue @teast , and for your proposed solution. I've noticed a couple of things wrong with the NUnit specflow files that need sorting out also.

There is one hiccup with your solution, given that the TestFixture attribute isn't required in later version of NUnit, so I'll have a play around and see if there is another solution that will catch both Nunit cases correctly, with or without the TestFixture in place.

Issafalcon commented 9 months ago

@teast - Hopefully this fixes the issue. Let me know if it hasn't.

teast commented 9 months ago

@Issafalcon It seems to have solved it. I tested it out in a solution that has both specflow and unit tests with nunit and all tests seems to be picked up and marked accordingly to their result. I also tested to run a single speclfow and unit test and there was no problem there either.

Great work on the plugin and thank you for a fast reply and fix! :)

Issafalcon commented 8 months ago

You're welcome @teast . It makes it easier when people come with decent reproduction steps, and especially when they come with a proposed solution like yourself!

marvindore commented 7 months ago

I am experiencing this issue as well and have tried modifying the treesitter query but don't quite have a handle on the syntax yet. I am using the following sample test:

using NUnit.Framework;

[TestFixture]
public class CalculatorTests
{
    [Test]
    [TestCase(1, 2, 3)]  // Test case 1: 1 + 2 = 3
    [TestCase(-1, -2, -3)]  // Test case 2: -1 + (-2) = -3
    [TestCase(0, 0, 0)]  // Test case 3: 0 + 0 = 0
    public void Add_TwoNumbers_ReturnsSum(int a, int b, int expectedSum)
    {
        // Arrange
        Calculator calculator = new Calculator();

        // Act
        int result = calculator.Add(a, b);

        // Assert
        Assert.That(expectedSum, Is.EqualTo(result));
    }
}

public class Calculator
{
    public int Add(int a, int b)
    {
        return a + b;
    }
}

Which creates duplicate test names as can seen here: image

EDIT: The Conflicting query seems to be coming from nunit-queries.lua, due to this test having two attributes [Test] and [TestCase] the method name from this query appears to be returned as well.

;; Matches test methods
    (method_declaration
      (attribute_list
        (attribute
          name: (identifier) @attribute_name (#eq? @attribute_name "Test" "TestCaseSource" ]] .. custom_test_attributes .. [[)
        )
      )
      name: (identifier) @[test.name](http://test.name/)
    ) @test.definition

I must note even after removing this to run my test, I only get the output reading:

Determining projects to restore...

All projects are up-to-date for restore.

So I must still have some form of setup issue.

Issafalcon commented 7 months ago

Hi @marvindore ,

Thanks for raising the issue!

This is actually an issue that is unrelated to the issue that this ticket was raised for.

Unfortunately, there are limitations in what treesitter queries can do, and this scenario, is one which cannot be solved (at least not with a decent amount of effort and more treesitter expertise than I have) by treesitter queries alone. The only way to avoid this duplication is to remove the [Test] attribute from your test methods that also have [TestCase] attributes. This won't affect the functionality of your tests, and will allow it to continue working using neotest-dotnet. I will add something to the documentation to outline this limitation with NUnit.

marvindore commented 7 months ago

Totally understand, thank you so much for developing this plugin and your continued support.