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

Update AutoFixture.NUnit2 to version depend on NUnit 2.6.4 #779

Closed
ladeak opened this issue Jul 11, 2017 · 15 comments
Closed

Update AutoFixture.NUnit2 to version depend on NUnit 2.6.4 #779

ladeak opened this issue Jul 11, 2017 · 15 comments
Labels

Comments

@ladeak
Copy link

ladeak commented Jul 11, 2017

Please update dependencies of AutoFixture.NUnit2 to use NUnit 2.6.4.

@zvirja
Copy link
Member

zvirja commented Jul 11, 2017

Could you please clarify a purpose of this change?

@ladeak
Copy link
Author

ladeak commented Jul 11, 2017

I am looking for a way to run AutoFixture tests with NUnit Test Adapter 2.1.1.0.
NUnit Test Adapter 2 is using NUnit 2.6.4, while AutoFixture.NUnit 2 still depends on 2.6.2 (as far as I see).

When I replace NUnit Test Adapter 2 with R#, where I can specify a the NUnit runner and I provide a matching (in version) nunit.core, it works. This gives me an idea that is fails run tests because of mismatch of the library versions. Currently the result of the test run is: "No arguments were provided".

Tried specifying assembyBindings, but that does not seem to help.

@zvirja
Copy link
Member

zvirja commented Jul 11, 2017

Could you please share more detail? Please specify:

  • VS Version
  • Do you use old or a new .NET Core SDK?
  • Which version of NUnit do you use in your project?
  • Could you share a simple sample with test that fail?

It will help to reproduce the issue locally and better understand the reason.
Thanks.

@ladeak
Copy link
Author

ladeak commented Jul 11, 2017

VS 2017
I am using standard .net framework 4.6.1
NUnit 2.6.4

An example:

[Test, AutoData]
public void Sample(int a)
{
  Assert.IsTrue(true);
}

@zvirja
Copy link
Member

zvirja commented Jul 11, 2017

Thanks for the details.

Previously we had the exactly same issue and there the decision was to keep it as is. However, we could probably change that in v4 if there is a real value from that change and no other obvious workaround is present.

I've quickly tried to update dependencies for our integration and test, however that doesn't fix the issue for me - VS runner still shows the No arguments were provided error.

I don't know too much about NUnit2 and don't use it, so probably @gertjvr or somebody from @AutoFixture/core could help to understand why our add-in doesn't seem to work more in 2.6.4.

@zvirja
Copy link
Member

zvirja commented Jul 22, 2017

After I played more with that I found that upgrading to nunit 2.6.4 actually fixes the issue and R# is capable of running tests out of the box. Also it seems to enable VS discover (while run still doesn't work like that is described here).

@ladeak Could you please test these artifacts where I updated NUnit to 2.6.4 and let me know whether it works fine for you?

Thank you in advance.

P.S. Notice, that test branch is based on upcoming v4, so you might observe some breaking changes in API when you install that NuGet packages.

@ladeak
Copy link
Author

ladeak commented Jul 24, 2017

Thank you for the follow up.
My current environment restricts me testing these artifacts, let me return to you soon.

@zvirja
Copy link
Member

zvirja commented Aug 6, 2017

I've investigated this question further (debugged the NUnit) to understand the reason and found that likely the only way will be to upgrade to the NUnit 2.6.4 to support it. The reason is that the LocalAddin we install to user's project is not recognized. I've investigated why that happens.

Basically, the init sequence is following (see here):

  1. Test assembly is loaded using the Assembly.Load(path) method.
  2. The assembly.GetExportedTypes() is called (see here) to iterate over all the public types in the assembly.
  3. If particular type is decorated with NUnitAddinAttribute attribute, that addin is installed.

If user uses NUnit 2.6.4 and installed our integration (which targets 2.6.2), the addin is not recognized. That is because on the step 1-2 when .NET resolves dependencies of the user's test assembly, it loads nunit.core.interfaces.dll 2.6.2 that we ship in addition to the 2.6.4 that is already in the memory (was loaded as test framework itself depends on it). Later, on step 3, when NUnit looks for the NUnitAddinAttribute, it doesn't find it as the attribute from the test assembly has a different identity (it's from a different assembly).

I've tried to add binding redirection as described here, but that does't help. It seems that Assembly.Load(assemblyPath) doesn't take binding redirection defined in the assemblyPath into account.

Also I've tried to add a Module initializer to redirect load to the nunit.core.interfaces.dll 2.6.4 that is already in the memory (so that 2.6.2 is never loaded), however module initializer is called too late :(

I wasn't able to find a way to make NUnit 2.6.4 work together with our integration (I've tested that with both Console runner and R#) and, therefore, would vote to re-target to 2.6.4 in v4. That version is about 3.5 years old and is used everywhere (as it was mentioned here), so that should not be an issue.

@AutoFixture/core what do you think? 😉

@ladeak
Copy link
Author

ladeak commented Aug 6, 2017

Thank your for the very detailed investigation.

@moodmosaic
Copy link
Member

@AutoFixture/core what do you think? 😉

Before AutoFixture.NUnit2 was created, back when we were discussing about it, I had my concerns, so I can't be of help in this space since I haven't followed this Glue Library's progress.

@zvirja
Copy link
Member

zvirja commented Aug 6, 2017

It took me 2 days to understand that I made a mistake in binding redirection and actually it helps to solve the issue 😅 This how the working App.config file looks on my machine:

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
  <runtime>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="nunit.core.interfaces" publicKeyToken="96d09a1eb7f44a77" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-2.6.4.14350" newVersion="2.6.4.14350" />
      </dependentAssembly>
      <dependentAssembly>
        <assemblyIdentity name="nunit.core" publicKeyToken="96d09a1eb7f44a77" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-2.6.4.14350" newVersion="2.6.4.14350" />
      </dependentAssembly>
    </assemblyBinding>
  </runtime>
</configuration>

With such config file tests are successfully passing with both Console and R# runners.

Therefore, we actually need to do nothing as there is a way to easily work around the issue and make it compatible with NUnit 2.6.4. Likely, we need to update the Readme.txt file in the AutoFixture.NUnit2 package to indicate this. I'll fire a PR later.

@ladeak Could you please check you how your binding redirection is defined and check again whether it works?

@moodmosaic I have a plan to refactor the glue libraries to extract the common parts (this is what I got from that boring referenced discussion 😇) . Rule of 3 should apply here as we have 4 libraries 😉 I'll fire a separate discussion a bit later.

@moodmosaic
Copy link
Member

@moodmosaic I have a plan to refactor the glue libraries to extract the common parts (this is what I got from that boring referenced discussion 😇) Rule of 3 should apply here as we have 4 libraries 😉 I'll fire a separate discussion a bit later.

Please don't, if you want my opinion.

  • In NUnit #158, we were actually trying to avoid doing something like this
  • The rule of three in this case should really be rule of nine, or more
  • It might end up being a facade for a bundle of libraries. Is it a good idea?
  • AutoFixture should, in my opinion, focus on the core itself instead of throwing huge maintenance amount of work on its Auto-mocking Containers and unit-testing Glue Libraries
  • You can't actually define such a hypothetical library because "clients [...] own the abstract interfaces" (APPP, ch. 11)
  • ...these are off the top of my head but I could probably also come up with even more reasons for not spending time on this

@ladeak
Copy link
Author

ladeak commented Aug 7, 2017

I am testing this again.
I have NUnit Test Adapter 2.1.1.0 extension installed to VS 2017.
I added NUnit 2 (nunit.framework and nunit.core.interfaces 2.6.4.14350) as reference.
I also added the suggested app.config.
Having Autofixture 3.50.3 with RhinoMocks 3.6.

Here is what I find. I ran these tests:

[Test, AutoData]
public void Sample(int a)
{
  Assert.IsTrue(true);
}

[Test, AutoData]
public void Sample2(bool a)
{
  Assert.IsTrue(a);
}

The one with bool input parameter runs correctly, and shows up as passed.
The one with int also shows up in Test Explorer window as 'Sample(130)', but after running I see the following output:
"Test adapter sent back a result for an unknown test case. Ignoring result for 'Sample(228)'"

See the value showing up in test explorer does not match with the value being used for the actual test run.

@zvirja
Copy link
Member

zvirja commented Aug 7, 2017

@ladeak That means that the original issue posted here was actually solved. You were able to run tests using the never version of NUnit2.

The issue you currently observe is related to the similar issue for NUnit3 (see #709). We should investigate whether we can do something similar with NUnit2. In any case, could you please create a separate issue for that? That is to make the track simpler.

In the meanwhile you can try to use different runners (like R#) that should not suffer from this issue.

@moodmosaic Thanks for that points! I need some time to realize them entirely 😳 If I finally decide to raise an issue, I'll keep them in mind!

@ladeak
Copy link
Author

ladeak commented Aug 8, 2017

Yes, I am fine closing this. Thank you for the help.

@ladeak ladeak closed this as completed Aug 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants