GustavEikaas/easy-dotnet.nvim

bug: Test result parser returns error if there is multiple tests with the same testId

Closed this issue · 8 comments

Hi!

Platform

Windows 11
Project with .NET 5.0

Description

In some projects with older .NET versions, VSTest can produce a .trx file with duplicated testId values (the executionId will be different). For more details, please check this link and this one.

Although this is not a problem with the plugin, it would be nice if the plugin could produce results with duplicated rows instead of no results at all.

I made a mistake in the second link; it should be this one

I read through the links you sent but I need more context.

Is this scenario appropriate:

  • Your project has a test using MemberData/ClassData/InlineData resulting in multiple test cases having the same TestId
  • These tests appear in the testrunner correctly?
  • When running these tests there is an error resulting in no status being reported and the test is stuck in ?
  • The test_parser F# script is the component that fails during test parsing?

I tested it again to collect more information and noticed some weird test cases in tests with duplicated IDs. It turned out there is an easy way to reproduce the issue:

  1. Create a new project with .NET 8.
  2. Add the latest NUnit and NUnit3TestAdapter:
    <PackageReference Include="NUnit" Version="4.2.2" />
    <PackageReference Include="NUnit3TestAdapter" Version="4.6.0" />
  3. Write a test with duplicated TestCase attributes:
    [TestFixture]
    public class Tests
    {
        [Test]
        [TestCase(1)]
        [TestCase(1)]
        public void TestFunc(int testArg)
        {
            ClassicAssert.AreEqual(1, testArg);
        }
    }

This will produce a .trx file with duplicated testIds

Here are the answers to your questions:

  1. As mentioned before, it's because of duplicated TestCase attributes (we had a big repository merge recently, so there are a lot of duplicates).
  2. Duplicated test cases appear as a single test case. This is correct, I guess, but on the other hand, Rider shows them as different test cases with a number next to the name:
    TestFunc(1)
    TestFunc(1) [2]
    
  3. It ends up with <No status reported>.
  4. Yes, if I run it manually via the command line, it logs an error like:
    Error: Cannot add property 3cb3c831-1c7a-ab6a-741a-f85b845af702 to Newtonsoft.Json.Linq.JObject. Property with the same name already exists on object.
    
    This is because the script creates a JObject with properties where the property name equals the testId and you can't have a JObject with duplicated properties

I see, I created a new project locally and verified your repro.

The way I see it the only way you can get duplicate TestIds are if your tests cases are identical in which case the result should also be 100% identical. If this is the case then we can simply add a check in the F# script and ignore duplicates. In all cases this should yield the correct results. As an extra precaution we can when we encounter duplicates check that the result of the testrun is the same.

The reason I cant rely on executionId is that I need a mapping between the discovered tests and their results from the trx file. Today im using testId. If I were to use executionId I could handle duplicates but it would be random which duplicate entry would be assigned which result. Which shouldnt matter because their results should infact be identical

Am I missing something or do you agree that duplicates are infact identical in the case that their result would always be the same?

There is still room for rare cases when a test can have duplicated test cases and be composed incorrectly at the same time; for example, it may depend on random or asynchronous operations that are not awaited, so one run can be successful while another can fail. The chances are pretty small, but they are still there. 😅

I see two solutions right now:

  1. Handle these cases like Rider: use separate rows for duplicated test cases. In my opinion, this is overkill for such edge case, but it is still a reasonable workaround.
  2. Show only one test case but check all executions. If there are any failures, then show "Failed" I believe there is no chance of having one Passed and another Skipped/etc."

Well it seems like we are on the same page. IMO if in that case two test executions report different results you have a flaky test. But yes theoretically I guess it could happen.

Personally I would have liked it to throw an error if it encountered a duplicate with a different result but I guess its not this plugins job to point out flaws in other peoples projects, so aggregating the result would probably be for the best.

Aggregating the result would be easy to do in the F# script. Just add the result and if you encounter a duplicate, check whether it should take precedence over the previous one.

Would you want to update your PR based on this?

Yeah, sure, I'll update PR in the next few days when I'll have spare time. Thanks for productive discussion!

PR updated