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

Any TestFixture deriving from a base-class which defines unit-tests will fail when the base-class is from another class-library #314

Closed
gimpf opened this issue Apr 10, 2017 · 14 comments · Fixed by #315

Comments

@gimpf
Copy link

gimpf commented Apr 10, 2017

We use some base-classes (from a common util-assembly) defining common tests (like correct implementation of comparison/equality/etc.), and use derived tests for specific types (which extend on the base tests).

This worked until 3.0.10 of the test-adapter, but got broke.

To illustrate this I've added a minimal sample below.

Expected behavior is that the test-adapter finds and runs two test-cases for CommonTests, namely TestCaseApplicableToAllTypes and SpecificTest.

Actual behavior is that only SpecificTest is discovered, but no test in the assembly containing class SpecificTypeTests is executed.

3.0.10 found the test-cases from CommonTests, but added them to a group/project External. Not perfect, but at least it executed the tests.

Code Sample
Needs two projects, both referencing NUnit (I'm still at 3.2.1, but should happen with newer version as well).

Project 1 a.k.a. "TestUtils"

namespace TestUtils {
    using NUnit.Framework;

    public abstract class CommonTests {
        [Test]
        public void TestCaseApplicableToAllTypes() { }
    }
}

Project 2 a.k.a. "Tests"

namespace Nunit3TestAdapterProblems {
    using NUnit.Framework;
    using TestUtils;

    [TestFixture]
    public class SpecificTypeTests : CommonTests {
        [Test]
        public void SpecificTest() { }
    }
}
@gimpf
Copy link
Author

gimpf commented Apr 10, 2017

BTW, the cause seems to be that NavigationDataProvider.GetNavigationData fails when trying to resolve the base-type in the other assembly via typeDef = typeDef.BaseType.Resolve();.

@CharliePoole
Copy link
Member

Are both assemblies in the same directory?

I assume that the console runner handles this correctly.

@gimpf
Copy link
Author

gimpf commented Apr 10, 2017

Both assemblies are in the same directory, but neither the console nor the GUI runner handle the case correctly. I also wouldn't expect any difference in behavior, as the resolve code is part of Mono.Cecil, and the .NET infrastructure for resolving assemblies is not part of it at all.

From the pull-request I have linked a test project, including a batch-file which illustrates the issue using the console runner.

@CharliePoole
Copy link
Member

Actually, the answer is useful because Mono.Cecil is used twice, by the engine for general purposes and by the adapter in order to locate the file in the source code. We have recently fixed several errors in this area so I'll have to check them as well as your PR.

@gimpf
Copy link
Author

gimpf commented Apr 10, 2017

Thanks for the heads-up. In case your time is limited: I could continue working on this tomorrow if you can point me in the right direction.

@OsirisTerje
Copy link
Member

This is equal to the issues we had in the NUnit2 adapter, see nunit/nunit-vs-adapter#147 and nunit/nunit-vs-adapter#143 , and was resolved there with the PR nunit/nunit-vs-adapter#149

At that time I checked it with NUnit3, which worked. However, I checked only with the vsix adapter, and I now tested it with the nuget adapter, and found there are issues.
I have uploaded a repro solution, https://github.com/OsirisTerje/NUnitTestAdapterIssue143 which was used to debug the NUnit2 adapter issues , and in the branch NUnit3 which references NUnit3 ,and I have added the NUNit3 test adapter too. There are 5 tests all together, and if I check with the nuget adapter, I get only 3:

image

If I check with the vsix, I get all 5.
image

When I say 5, that is included the fact that one test repeats twice, once for the base class and once for the derived class. That test also can't jump to the source code.
If I make the test abstract I get only 4 tests, but I cant jump to the test in the base class.

image

Take a look at the PR and the navigation fix there. I noted at the time that it wasn't like that in the NUnit3 adapter, and we assumed it was related to the fact that NUnit3 adapter already has the mono.cecil dlls, but it seems there are more to this issue.

@CharliePoole
Copy link
Member

Yes, this has a lot of confusing bits to it, not the least of which is that the user reports it also fails to work correctly in the console runner. However, that may depend on the version of runner plus engine used. I mistakenly assigned this to myself using the PR and made some comments there. I'd like to clarify where the problem lies: engine, adapter or both.

Regarding use of abstract for the base class - that is the correct way to do it as we have always advised. In fact, the general unit test pattern that this exemplifies is usually called the "Abstract Fixture" pattern. Using a non-abstract base class in NUnit usually means that the tests in the base will be included twice, which is not what is normally desired. In this case, the fact that the base is in a separate assembly changes that behavior.

I'll try out your example, first under various engines and then under the adapter.

@CharliePoole
Copy link
Member

@OsirisTerje On reviewing the code, it seems that your example is not quite the same as @gimpf 's.

He has a concrete test class, which inherits from an abstract base. Tests in the abstract base should be run as a part of the concrete base.

Your example has that situation but it also has classes that only exist in the "base" assembly and are never referenced from the test assembly. Those classes would normally never be seen by NUnit.

In "normal" operation, the user gives NUnit a list of assemblies from which to run tests, e.g.:

nunit3-console test-assembly.dll

In that situation, only tests in the assembly and tests defined in base classes in other assemblies would be seen by NUnit.

However, when using the adapter, VS passes all the assemblies to NUnit. It's as if the user had typed

nunit3-console test-assembly.dll other-assembly.dll

In that case, NUnit tries to run anything that looks like a test that is found in the other assembly.

I'm going to start by working with @gimpf 's example and then continue with yours. I think we may have found an area where added guidance to users is needed.

@CharliePoole
Copy link
Member

CharliePoole commented Apr 11, 2017

@gimpf I've tested your case with the nunit3-console runner 3.0, 3.2, 3.5, 3.6 and the current 3.6.1. It detects two tests correctly in all cases. I agree that we have a problem in the adapter but I can't find any more fundamental problem that would cause the console runner to fail. Are you sure of your results? What versions and command-line did you use?

@gimpf
Copy link
Author

gimpf commented Apr 11, 2017

Argl, there's been a misunderstanding... To illustrate the issue I was using the test-adapter, via the visual studio console runner, not the NUnit console runner. I haven't tried the latter at all...

@gimpf
Copy link
Author

gimpf commented Apr 11, 2017

An example of the invocation is in nunit3-vs-adapter-issue314/execute-tests.bat:

vstest.console.exe /TestAdapterPath:..\nunit3-vs-adapter\bin\Debug unit-tests.dll

@gimpf
Copy link
Author

gimpf commented Apr 11, 2017

@OsirisTerje I've just read through your PR and the related Mono.Cecil code. Your PR is strictly more general than what I wrote (I didn't know that a module-definition can contain types from modules with other paths).

How do you share code between those projects? Should I update my PR, or do you have another system in place?

@OsirisTerje
Copy link
Member

OsirisTerje commented Apr 11, 2017

@gimpf You have to update your PR, as we don't have any way of sharing code between the two projects. (The NUnit2 should be obsolete once we get the NUnit3 adapter to cover NUnit2 tests too)
Also note that I check that the assembly is not being added multiple times, since Mono.Cecil doesn't do that check itself - I checked the source for Mono.Cecil for just that.

@OsirisTerje OsirisTerje added this to the 3.8 milestone Jul 14, 2017
@jnm2
Copy link
Contributor

jnm2 commented Mar 13, 2018

BTW, the cause seems to be that NavigationDataProvider.GetNavigationData fails when trying to resolve the base-type in the other assembly via typeDef = typeDef.BaseType.Resolve();.

Note: We removed the adapter's dependency on Mono.Cecil in #454. That change was released in https://www.nuget.org/packages/NUnit3TestAdapter/3.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants