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

Test cases are skipped with TestCaseSource under Visual Studio 2019 #676

Open
LazarLashev opened this issue Oct 18, 2019 · 13 comments

Comments

@LazarLashev
Copy link

@LazarLashev LazarLashev commented Oct 18, 2019

Running latest versions of Visual Studio 2019 and NUnit (Visual studio 2019 16.3.5, NUnit 3.12.0, NUnit3TestAdapter 3.15.1)

Test with a test source:

[Test]
[TestCaseSource(nameof(TestCases))]
public async Task When_InvalidRequest_ReturnsValidationError(PlayerBalancesRequest request, string siteCode, string correlationToken)
{
   ...
}

public class PlayerBalancesRequest
{
    public long PlayerId { get; set; }
    public string Token { get; set; }
    public string CurrencyCode { get; set; }
}

private static object[] TestCases()
{
    return new object[]
    {
        new object[] { new PlayerBalancesRequest(), null, null },
        new object[] { new PlayerBalancesRequest { PlayerId = 0, Token = GameToken, CurrencyCode = CurrencyCode }, SiteCode, CorrelationToken },
        new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = null, CurrencyCode = CurrencyCode }, SiteCode, CorrelationToken },
        new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = GameToken, CurrencyCode = null}, SiteCode, CorrelationToken },
        ...
    };
}

I have 15 test cases in the test case source. Visual Studio shows all of them in the Test Explorer, but NUnit always runs just 3 of them. Always the same 3, and the rest are skipped. If I remove these 3, then it runs no tests. As a separate issue, if I leave just 1 test case in the test case source, it will think this object[3] that is returned are 3 separate test cases (PlayerBalancesRequest, string and string) and will run the test 3 times with just 1 parameter, failing because the test requires 3 parameters.

@CharliePoole

This comment has been minimized.

Copy link
Member

@CharliePoole CharliePoole commented Oct 18, 2019

You have only shown four of your test cases, but even with that small number, I can see that three of them will generate tests with the same name. That's the source of your problem.

There's a mismatch between who NUnit works and how TestExplorer wants to see tests. NUnit doesn't care if multiple tests have the same name, because it doesn't use the name to identify the tests. TestExplorer expects each test to have a unique name. The NUnit test adapter does nothing to ensure this. It merely passes on the name provided (usually implicitly) by the user.

In your case, the first argument PlayerBalanceRequest has a default string representation - IIRC <PlayerBalanceRequest>. The second and third args areinsufficient to provide uniqueness. For the four cases you give above, the two names generated will be something like the following, substituting the actual string values for SITECODE and TOKEN.

When_InvalidRequest_ReturnsValidationError(<PlayerBalanceRequest>, null, null)
When_InvalidRequest_ReturnsValidationError(<PlayerBalanceRequest>, "SITECODE", "TOKEN")

Generally, when tests have the same name, TestExplorer groups all the results under the first one found. Select one of the tests that ran and look at the result report, usually directly under the tree but sometimes next to it if your window size is small. I suspect you'll find all fifteen.

So, what can you do about this? There are a two main alternatives...

  1. Override PlayerBalanceRequest.ToString() so the representation includes the values of all the member properties. This will give you unique names for each case, so long as there are no duplicates - I assume you wouldn't want duplicates anyway.

  2. Replace the inner new object[]s with new TestCaseData() and use TestCaseData.SetName to assign a unique name to each case. If you do this, I advise replacing the outer new object[] with new TestCaseData[] because it's much clearer. Personally, I believe that use of TestCaseData is a better practice in general, because it clearly says: "Here is one set of test case data!" and because you can easily add features like ignoring a case, changing the name, providing a description, etc.

Regarding the "separate issue" you mentioned, Did you eliminate the outer object[]? If you did that, it's a user error. 😄 If you retained the nesting, then it might be a bug in the framework. In either case, the problem goes away if you use TestCaseData instead.

@CharliePoole

This comment has been minimized.

Copy link
Member

@CharliePoole CharliePoole commented Oct 18, 2019

@nunit/framework-team Maybe we should consider reducing the number of options available for use with TestCaseSource for NUnit 4. I'd be happy to actually require TestCaseData items under a source. The only possible exception in my mind is for a method with a single argument. In that case, allowing an array of the declared type to represent the list of args is more terse. However, it opens up problems when the type of the single argument is, in fact, some sort of array! I've often regretted trying to make this so flexible in the first place.

@LazarLashev

This comment has been minimized.

Copy link
Author

@LazarLashev LazarLashev commented Oct 18, 2019

Thanks for the detailed explanation @CharliePoole! I will try your suggestions and report the results. However, I don't think the issues can be fully explained by this theory. Here are all the test cases:

// NUnit runs these:
new object[] { new PlayerBalancesRequest(), null, null },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = GameToken, CurrencyCode = CurrencyCode }, SiteCode, null },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = GameToken, CurrencyCode = CurrencyCode }, SiteCode, string.Empty },

// NUnit skips these:
new object[] { new PlayerBalancesRequest { PlayerId = -1, Token = GameToken, CurrencyCode = CurrencyCode }, SiteCode, CorrelationToken },
new object[] { new PlayerBalancesRequest { PlayerId = 0, Token = GameToken, CurrencyCode = CurrencyCode }, SiteCode, CorrelationToken },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = null, CurrencyCode = CurrencyCode }, SiteCode, CorrelationToken },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = string.Empty, CurrencyCode = CurrencyCode }, SiteCode, CorrelationToken },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = new string('*', 51), CurrencyCode = CurrencyCode }, SiteCode, CorrelationToken },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = GameToken, CurrencyCode = CurrencyCode }, null, CorrelationToken },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = GameToken, CurrencyCode = CurrencyCode }, string.Empty, CorrelationToken },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = GameToken, CurrencyCode = CurrencyCode }, new string('*', 51), CorrelationToken }

As you can see there are various test cases where the site code and correlation token are different, but these test cases will still be ignored. I have put the ones NUnit runs in the beginning, but the ordering does not matter; it only ever runs these 3.

For example, if I leave only the last 3 test cases, they will all be ignored and no test cases will be run at all.

new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = GameToken, CurrencyCode = CurrencyCode }, null, CorrelationToken },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = GameToken, CurrencyCode = CurrencyCode }, string.Empty, CorrelationToken },
new object[] { new PlayerBalancesRequest { PlayerId = PlayerId, Token = GameToken, CurrencyCode = CurrencyCode }, new string('*', 51), CorrelationToken }
@CharliePoole

This comment has been minimized.

Copy link
Member

@CharliePoole CharliePoole commented Oct 18, 2019

Hmmm... that messes up my theory all right! What platform do your tests target? If it's .NET Framework, can you check what happens if you run them under the nunit3-console runner?

@mikkelbu

This comment has been minimized.

Copy link
Member

@mikkelbu mikkelbu commented Oct 18, 2019

Is there any warnings/errors under the output for test execution? And can you upload the example or create a github repository that we can clone, so that we can recreate the problem easily.

@LazarLashev

This comment has been minimized.

Copy link
Author

@LazarLashev LazarLashev commented Oct 21, 2019

Good morning, here is a small sample project that illustrates the problem:
NUnitTest.zip

If I run the tests through the console runner (in our case through TeamCity) it does run all tests, so the problem is related to the tests adapter.

No errors or warnings in the output window!

@mikkelbu

This comment has been minimized.

Copy link
Member

@mikkelbu mikkelbu commented Nov 3, 2019

@LazarLashev What is the outcome if you use https://www.myget.org/feed/nunit/package/nuget/NUnit3TestAdapter/3.16.0-dev-01202 instead of NUnit3TestAdapter 3.15.1? (The build contains a rather substantial change to how the adapter identifies test cases, see #668 for a longer - and better - explanation)

@APErebus

This comment has been minimized.

Copy link

@APErebus APErebus commented Nov 6, 2019

I was having a similar issue with VS 2019 & TestCaseSource. The dev 3.16.0 test adapter seems to have resolved the issue.

Test Explorer can now find and run my tests.

@LazarLashev

This comment has been minimized.

Copy link
Author

@LazarLashev LazarLashev commented Nov 6, 2019

@mikkelbu I have taken the sample test project above (NUnitTest.zip) and ran the tests with the updated adapter 3.16.0-dev-01202. It failed to run the tests, showing this output:

[11/6/2019 9:57:55.396 AM Informational] ---------- Discovery started ----------
[11/6/2019 9:57:55.398 AM Informational] ========== Discovery skipped: All test containers are up to date ==========
[11/6/2019 9:57:55.404 AM Informational] ---------- Run started ----------
[11/6/2019 9:57:55.797 AM Error] System.ArgumentException: An item with the same key has already been added.
   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
   at Microsoft.VisualStudio.TestWindow.Client.VirtualTestNodeHierarchy.<>c__DisplayClass40_1.<<RefreshTestsAsync>b__1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Utilities.EventPumpExtensions.<>c__DisplayClass3_0.<<EnqueueAsync>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Extensibility.ILoggerExtensions.<CallWithCatchAsync>d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)
[11/6/2019 9:57:56.108 AM Informational] NUnit Adapter 3.16.0.0: Test execution started
[11/6/2019 9:57:56.123 AM Informational] Running selected tests in D:\Projects\Test\NUnit\bin\Debug\netcoreapp2.2\win-x64\NUnitTest.dll
[11/6/2019 9:57:56.383 AM Informational]    NUnit3TestExecutor converted 11 of 11 NUnit test cases
[11/6/2019 9:57:56.566 AM Informational] NUnit Adapter 3.16.0.0: Test execution complete
[11/6/2019 9:57:56.661 AM Informational] ========== Run finished: 11 tests run (0:00:01.2205531) ==========
[11/6/2019 9:57:56.856 AM Error] System.ArgumentException: An item with the same key has already been added.
   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
   at Microsoft.VisualStudio.TestWindow.Client.VirtualTestNodeHierarchy.<>c__DisplayClass40_1.<<RefreshTestsAsync>b__1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Utilities.EventPumpExtensions.<>c__DisplayClass3_0.<<EnqueueAsync>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Extensibility.ILoggerExtensions.<CallWithCatchAsync>d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)
@lobster2012-user

This comment has been minimized.

Copy link

@lobster2012-user lobster2012-user commented Nov 7, 2019

@LazarLashev
Have you tried to completely clean the project? (remove obj \ bin)

изображение

изображение

@LazarLashev

This comment has been minimized.

Copy link
Author

@LazarLashev LazarLashev commented Nov 7, 2019

I have tried to run the tests after removing bin/obj, and the first time they all passed. The second time I tried to run the tests, I got what it seems like the same error as before

[11/7/2019 5:08:47.041 PM Informational] ---------- Discovery started ----------
[11/7/2019 5:08:47.050 PM Informational] ========== Discovery skipped: All test containers are up to date ==========
[11/7/2019 5:08:47.085 PM Informational] ---------- Run started ----------
[11/7/2019 5:08:47.404 PM Error] System.ArgumentException: An item with the same key has already been added.
   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
   at Microsoft.VisualStudio.TestWindow.Client.VirtualTestNodeHierarchy.<>c__DisplayClass40_1.<<RefreshTestsAsync>b__1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Utilities.EventPumpExtensions.<>c__DisplayClass3_0.<<EnqueueAsync>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Extensibility.ILoggerExtensions.<CallWithCatchAsync>d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)
[11/7/2019 5:08:47.766 PM Informational] NUnit Adapter 3.16.0.0: Test execution started
[11/7/2019 5:08:47.785 PM Informational] Running selected tests in D:\Projects\Test\NUnit\bin\Debug\netcoreapp2.2\win-x64\NUnitTest.dll
[11/7/2019 5:08:48.189 PM Informational]    NUnit3TestExecutor converted 11 of 11 NUnit test cases
[11/7/2019 5:08:48.410 PM Informational] NUnit Adapter 3.16.0.0: Test execution complete
[11/7/2019 5:08:48.544 PM Informational] ========== Run finished: 11 tests run (0:00:01.3926949) ==========
[11/7/2019 5:08:48.703 PM Error] System.ArgumentException: An item with the same key has already been added.
   at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
   at Microsoft.VisualStudio.TestWindow.Client.VirtualTestNodeHierarchy.<>c__DisplayClass40_1.<<RefreshTestsAsync>b__1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Utilities.EventPumpExtensions.<>c__DisplayClass3_0.<<EnqueueAsync>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Extensibility.ILoggerExtensions.<CallWithCatchAsync>d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)
@LazarLashev LazarLashev closed this Nov 7, 2019
@LazarLashev LazarLashev reopened this Nov 7, 2019
@CharliePoole

This comment has been minimized.

Copy link
Member

@CharliePoole CharliePoole commented Nov 7, 2019

It appears as if the error is associated with the test window deciding to skip discovery and use information that it has saved. See the message "Discovery skipped: All test containers are up to date."

Immediately after that message, execution begins. NUnit, in the course of execution, performs it's own discovery once again. This should be private, with no results reported to VS, but it's possible that some reporting is happening. That could conceivably lead to the duplicate key error that is being thrown by the Test window.

In any case, this is clearly an error in the adapter only, so I'm transferring the issue to that project. @OsirisTerje does this match anything else you have seen?

@CharliePoole CharliePoole transferred this issue from nunit/nunit Nov 7, 2019
@OsirisTerje

This comment has been minimized.

Copy link
Member

@OsirisTerje OsirisTerje commented Nov 7, 2019

@CharliePoole Not directly, but after the last big related fix, I have at least 3 hanging bugs which should have been fixed by the same. I have asked the MS PG about this one in particular since the stack trace looks unknown to the adapter. I am 99.9% sure nothing is being sent out from the discovery phase with the execution, but we do build up a cache, so there can be something there.

I'll work through this one and see if I can catch a trace from it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.