Build/Test Issues

NUnit duplicate name warning without duplicate names

Started by ssg on 14,317 views

This test below gives the false positive of duplicate name warning (although parameters are different):


        [Test]
        [TestCase("1234567890 1234567890 1234567890 1234567890 AA", "1234567890")]
        [TestCase("1234567890 1234567890 1234567890 1234567890 BB", "1234567890")]
        public void ParameterizedTest(string a, string b)
        {
            StringAssert.Contains(b, a);
        }


But this test doesn't give any warnings. It looks like NCrunch only takes into first 30-40 chars of a parameter into account instead of all of it:


        [Test]
        [TestCase("1234567890 1234567890 1234567890 AA", "1234567890")]
        [TestCase("1234567890 1234567890 1234567890 BB", "1234567890")]
        public void ParameterizedTest(string a, string b)
        {
            StringAssert.Contains(b, a);
        }


NUnit console runner doesn't complain about these at all. What's the problem?

Thanks!

Sedat

Edited

Hi Sedat,

Thanks for sharing this. It looks like this problem is caused by NCrunch truncating the longer parameters to try and make the test names shorter and easier to read. It seems this is also impacting the test uniqueness (which it shouldn't).

I've noted this down to be addressed. There are a few problems at the moment with unique identification of NUnit test names, so hopefully I can address this with the others.

I recommend adjusting your parameterisation to avoid long parameters. In this situation the warning NCrunch is giving you isn't a false positive - its a symptom of an internal problem. When you see this warning, NCrunch won't be running all of the tests shown here.
Thanks Remco! Our tests exercise length boundaries so they have to be long. I'm currently just changing the prefixes but that's not always available for all tests (and hurts readability too). About when should we expect a fix to be released? This is currently the biggest issue about our switch from MBUnit to NUnit. Thanks!
And also because the duplicate test check uses `ToString()`, any parameter objects that don't have ToString() implementation shows up the object type instead and cause the same problem even without any strings.
ssg wrote:And also because the duplicate test check uses `ToString()`, any parameter objects that don't have ToString() implementation shows up the object type instead and cause the same problem even without any strings.


This is also true, although there may be less we can do about that :(
Remco wrote:This is also true, although there may be less we can do about that :(


shoot! although many classes only implement Equals/GetHashCode() instead of ToString() so using GetHashCode() might be a much better idea.

anyway before you wrote the answer i had written a POC, in case it helps:

        public class DummyClass
        {
            public int A { get; set; }
        }

        private static DummyClass[] testData = new DummyClass[]
        {
            new DummyClass { A = 1 },
            new DummyClass { A = 2 },
        };

        [Test]
        [TestCaseSource("testData")]
        public void FactoryTest(DummyClass a)
        {
            Assert.IsNotNull(a);
        }

Edited

In NCrunch 2.20.04 I'm receiving the warning below for the member-driven tests. The source code can be reached at https://github.com/ssg/HashDepot
I had been using xUnit.Net and switched to NUnit, then encountered this problem. So I don't know if it has always been there.

This project contains multiple NUnit tests that share the same name. This prevents NCrunch from uniquely identifying tests during their discovery and execution, distorting the reporting of test results. Ensure your tests always have a unique name within the scope of their project.

The following test names are duplicated in this project:

HashDepot.Test.Fnv1aTest.Hash32_ReturnsExpectedValues(HashDepot.Test.FnvTestVector)
HashDepot.Test.Fnv1aTest.Hash32_ReturnsExpectedValues(HashDepot.Test.FnvTestVector)
HashDepot.Test.Fnv1aTest.Hash32_ReturnsExpectedValues(HashDepot.Test.FnvTestVector)
HashDepot.Test.Fnv1aTest.Hash32_ReturnsExpectedValues(HashDepot.Test.FnvTestVector)
HashDepot.Test.Fnv1aTest.Hash32_ReturnsExpectedValues(HashDepot.Test.FnvTestVector)

Edited

Hi, thanks for sharing this issue.

I've been in discussions with Charlie about changes to NUnit to better handle test identification between discovery and execution sessions. The consensus we've arrived at is that test cases making use of user defined type parameters are likely to continue to be a problem in future, as we have no way of uniquely identifying the individual test cases across sessions. You'll notice that even visibly, there is no way of telling these test cases apart.

When working with user defined type parameters and TestCaseSource, it's important that you give each test case a meaningful name using NUnit's TestCaseData.SetName method.
Adding "ToString()" implementation to FnvTestVector class fixed the naming issue. No need to use SetName or anything else. I pushed the latest changes to the repository.
Now with the latest version of NCrunch (2.22.0.1) I'm getting the duplicate warning for this test for NUnit 3:

        [Test]
        [TestCase(10, 1, 1, 0)]
        [TestCase(10, 2, 1, 1)]
        [TestCase(10, 10, 1, 9)]
        public void SomeTest(int a, int b, int c, int d)


for the case: (10,2,1,1)

(The fixture is marked parallelizable)

EDIT: interesting "Resynchronize, Rebuild" made this issue go away. I expected it to go away by itself.

Edited

ssg wrote:
EDIT: interesting "Resynchronize, Rebuild" made this issue go away. I expected it to go away by itself.


Thanks for sharing this issue. Using the code sample you've provided, I haven't been able to reproduce it. If you can get this to happen again, would you mind submitting a bug report?

Do also know if you find a way to reproduce it.
Remco wrote:
Thanks for sharing this issue. Using the code sample you've provided, I haven't been able to reproduce it. If you can get this to happen again, would you mind submitting a bug report?


Thanks Remco. My diagnosis is: I received the error while I was writing the test (copying pasting lines) so when I coped the same TestCase line I got the error. Then I corrected the data but the error did not disappear. That should be trivial to reproduce right?

(I was in "Run impacted tests automatically" mode fyi)

Edited

ssg wrote:
Thanks Remco. My diagnosis is: I received the error while I was writing the test (copying pasting lines) so when I coped the same TestCase line I got the error. Then I corrected the data but the error did not disappear. That should be trivial to reproduce right?



Got it! Thanks!

The warning isn't being cleared when the analysis has a clear run. I'll make a note to get this fixed. Thanks for making me aware of it.
Hi,

My shop has been wrestling with the scenario ssg described, where the TestCaseSource points to a collection of class type. We use that pattern a lot.

We noticed that NUnit does have the same naming behavior, in that it also gives each member of the collection the same test name. We can live with that. What causes us some pain is the difference in how the test runners operate on these tests.

We use the NCrunch plugin for Visual Studio at our workstations, and we use the command line version for our TeamCity builds. We find that the NCrunch test runner will quietly skip all but the first test case in the collection (although we do see the duplicate name warnings in the NCrunch Tests pane in VS). As a result, we get green coverage dots in VS where most of our tests are actually failing to run, and deceptively clean build logs on the TC server.

If I load the test assembly in the NUnit GUI, I see the duplicate test names there. But the tests are still treated as distinct for the purposes of execution. So I see the appropriate pass/fail result for each when running the tests. The tests also run properly in the NUnit command-line program.

We enjoy the convenience of NCrunch, and so we'd prefer to continue relying on it. We'd also prefer to not have to override ToString() on all our TestCaseSource classes. Is another workaround possible to make NCrunch execute our "duplicate" tests the way NUnit does? Otherwise, might an upcoming release of NCrunch address this issue?
Hi,

Please see here for more details about this problem - http://forum.ncrunch.net/yaf_postst1799_Doesn-t-recognize-multiple-tests-with-same-name-using--ValueSource---NUnit.aspx.

In short, there is no workaround here. This is a problem with the test suite itself. Future changes to NUnit are likely to make it impossible to design tests in the way that you have done, as even internally within NUnit there is no way to tell them apart. The only solution is to redesign the tests.
To circumvent this issue, we started using
TestCaseData[]
instead of
object[]
. TestCaseData instances can be given individual names easily with its fluent syntax:


private static TestCaseData[] testData = new []
{
    new TestCaseData(item1, item2, item3).SetName("unique name for test 1"),
    new TestCaseData(item1, item2, item3).SetName("unique name for test 2"),
};


That removes the need to override ToString() in the actual code. I still wonder though, how NUnit could not have come up with a "sequence" based identification for data-driven tests? That would be more than enough between builds and debug attempts. We don't need a universally unique identifier, a sequence would work almost 99.999% of the time. MBUnit did not even support individual running of data driven tests. So it's nice to have the ability to execute/debug individual test cases, but I'm almost always ok with running the wrong test when I edit rows in the next build.
ssg wrote:
That removes the need to override ToString() in the actual code. I still wonder though, how NUnit could not have come up with a "sequence" based identification for data-driven tests? That would be more than enough between builds and debug attempts. We don't need a universally unique identifier, a sequence would work almost 99.999% of the time. MBUnit did not even support individual running of data driven tests. So it's nice to have the ability to execute/debug individual test cases, but I'm almost always ok with running the wrong test when I edit rows in the next build.


Sequenced based identification unfortunately doesn't work when trying to correlate state between test sequences.

Let's say, for example, you have 3 test cases each with the same name (I've given each a number suffix so we can tell them apart, but assume for the sake of the runner that they show the same)

TestCase1
TestCase2
TestCase3

And you run these test cases. The runner uses sequence IDs, which are used to map their results in NCrunch. Each test case then has coverage results, trace output, and a pass/fail flag.

Then, you choose to add a new test case to the top of the suite:

TestCase0
TestCase1
TestCase2
TestCase3

Now NCrunch runs your discovery step. When the discovery step has finished, suddenly, ALL the code coverage data collected from TestCase1 is now assigned to TestCase0. The same goes for the pass/fail result and the trace output. TestCase0 hasn't even been run yet, so this is technically very wrong. The other test cases likewise have their results entirely skewed.

I grant that in the majority of cases, you won't see this. Most likely you'll write the test, get the test working, then it'll sit in your suite and may never be updated again. So the 99.999% of the time estimation you've given may in fact be correct, but in the 0.001% that this isn't the case, we actually care very much, because NCrunch is now providing you with a result that is dangerously misleading.

You could make the argument that the coverage and pass/fail results don't matter between the individual test cases, but in this case, I would ask why you would be using TestCaseSource anyway. If the test cases cannot be make visually distinctive and can't have their own safely correlated result sets, why not just bundle them into a single test with a loop?

Remco wrote:
Now NCrunch runs your discovery step. When the discovery step has finished, suddenly, ALL the code coverage data collected from TestCase1 is now assigned to TestCase0.


This makes sense. I didn't think about code coverage. Thanks for the explanation!

Post a reply

Log in to reply.