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

Unable to run multiple test assemblies in a single app domain #189

Closed
alexjamesbrown opened this issue Nov 1, 2013 · 19 comments · Fixed by #196
Closed

Unable to run multiple test assemblies in a single app domain #189

alexjamesbrown opened this issue Nov 1, 2013 · 19 comments · Fixed by #196
Assignees
Labels
Milestone

Comments

@alexjamesbrown
Copy link

From SO post: http://stackoverflow.com/questions/19727000/unit-tests-failing-when-run-altogether-api-restriction-the-assembly-has-alrea

I've got a set of unit tests, which, if I run all together (using resharper) I get this error:

SetUp : System.TypeInitializationException : The type initializer for 'FakeItEasy.Core.FakeScope' threw an exception. ----> System.IO.FileLoadException : API restriction: The assembly 'file:///C:\Users\abrown\Documents\Repos\ink.services.jetstar\My.Namespace.Tests.Unit\bin\Debug\My.Namespace.dll' has already loaded from a different location. It cannot be loaded from a new location within the same appdomain.

If I run them individually, they pass.

Further down in the exception, it's failing on lines like this:

[SetUp]
public void SetUp()
{
_myFake = A.Fake();

As we were discussing on Jabbr (https://jabbr.net/#/rooms/fakeiteasy)

If I run the tests in parallel, they pass

@blairconrad
Copy link
Member

Reproduced. I just added FakeItEasy.Tests and FakeItEasy.IntegrationTests to a single NUnit project and give it a run. Investigations continue!

@ghost ghost assigned blairconrad Nov 1, 2013
@blairconrad
Copy link
Member

Running the tests using either one process per assembly or one AppDomain per assembly via NUnit are workarounds:
/domain=Multiple or /process=Multiple. These options don't run the tests in parallel - they just provide a fresh canvas, as it were. There are NUnit GUI options for this as well. I'm not sure about other test runners.
image

I'm not saying this is a fix. I'd prefer not to tell users how to run their tests, but am not yet sure how to resolve this. The thrown exception has a nice message, but is otherwise indistinguishable from other exceptions that might arise from loading assemblies.

As I suspected, the problem (at least when I reproduce it) occurs when we've already loaded an assembly in the current AppDomain and then we attempt to load a copy of the assembly from the current directory.

As I sit here, I'm not sure what technical path we should take. Some things we can do:

  1. Document it. Maybe grab exceptions thrown during load and wrap them with a slightly nicer message. This feels like the coward's way.
  2. Catch the loading exceptions. Ignore them. Hope it works out. We could possibly filter the message to see if it's this exact problem, or compare the path we're loading with already loaded assemblies (filename only) to see if this is the problem. It's risky, but may work.
  3. A while ago, I'd idly considered skipping loading assemblies that have the same filename as already-loaded assemblies, rather than comparing the whole path. Not as a reaction to an error, just as routine loading while scanning for modules. This would eliminate this problem as well as mean that we'd get a startup time boost even when shadow copies are turned on. The potential downside is that someone somewhere may have 2 same-filenamed assemblies that are actually different assemblies and at least one of them (I'd say both, but we can't guarantee which one will be skipped) defines a module such as an ArgumentValueFormatter or DummyDefinition and then we wouldn't load it when we should. I think the risk is low, but we don't have a good story around telling the user what's happening in this case, so that's a downside.
  4. Finally implement configurable assembly scanning, as in Client-supplied bootstrappers can override the default behaviour when scanning assemblies for extensions  #130. I'd think we'd want to combine this with better error-catching and reporting, though.

@FakeItEasy/owners, any opinions?

@blairconrad
Copy link
Member

@alexjamesbrown, just to be clear, you're running tests from multiple assemblies, yes? I thought that was what we discussed. If you run tests all from the same assembly, you don't have this problem?

@adamralph
Copy link
Contributor

@blairconrad thanks for the analysis!

  • Option 1 - I agree and I'm not in favour. I think we should try to make things work somehow.
  • Option 2 - this is my favourite right now. Am I correct in understanding that this is only affecting the loading of the custom argument formatters etc. for extending FIE? If so, I think it's safe to assume that if an assembly has already been loaded then we don't need to try and load it again. It is indeed a shame that all we get is a TypeInitializationException and not something like AssemblyAlreadyLoadedException which means we may have to parse the exception message, as ugly and horrible as that is. I assume there's no other special property of TypeInitializationException that can give us a clue that this is the problem? Or perhaps an exception in the .InnerException chain?
  • Option 3 - I think this is really just a cheap version of option 2. I prefer option 2; we should at least attempt to load assemblies with the same name and then handle any exception.
  • Option 4 - am I correct in understanding that this would require intervention to get things running? I.e. the test author would have to configure things in a way that solves the problem? If so I'd rather we investigated a solution which 'just works' without the need for intervention.

@alexjamesbrown
Copy link
Author

Sounds like we're making progress!
Glad it wasn't me just losing my mind! :)

I think we should document it (for now) - It is indeed the cowards way out,
but when I was struggling with this, I couldn't find anything to point me
in the right direction (hence the SO post)
I'll stick a v. quick blog up about it, and the quick work-around for now
while we come up with a more perm. solution

Thanks for your help

On 2 November 2013 12:44, Adam Ralph notifications@github.com wrote:

@blairconrad https://github.com/blairconrad thanks for the analysis!

Option 1 - I agree and I'm not in favour. I think we should try to
make things work somehow.

Option 2 - this is my favourite right now. Am I correct in
understanding that this is only affecting the loading of the custom
argument formatters etc. for extending FIE? If so, I think it's safe to
assume that if an assembly has already been loaded then we don't need to
try and load it again. It is indeed a shame that all we get is a
TypeInitializationException and not something like
AssemblyAlreadyLoadedException which means we may have to parse the
exception message, as ugly and as horrible as that is. I assume there's no
other special property of TypeInitializationException that can give us
a clue that this is the problem? Or perhaps an exception in the
.InnerException chain?
-

Option 3 - I think this is really just a cheap version of option 2. I
prefer option 2; we should at least attempt to load assemblies with the
same name and then handle any exception.

Option 4 - am I correct in understanding that this would require
intervention to get things running? I.e. the test author would have to
configure things in a way that solves the problem? If so I'd rather we
investigated a solution which 'just works' without the need for
intervention.


Reply to this email directly or view it on GitHubhttps://github.com//issues/189#issuecomment-27621179
.

@adamralph
Copy link
Contributor

@alexjamesbrown agreed, documenting until it is fixed is a very good idea. Looking forward to your blog post!

@patrik-hagne
Copy link
Member

Haven't had the time to totally wrap my head around this, but I agree with what @adamralph says about option 2. In general, failing to load "extensions" or "plugins" should never crash. It should not throw an exception, it should either fail silently or possibly log to the console. That's my two cents!

@adamralph
Copy link
Contributor

Good point, perhaps we don't need to bother trying to be clever with the exception at all then. Just throw into stdout and be done with it.

@blairconrad
Copy link
Member

Hi, @adamralph.

I'm late to the party. Ran out of the house with this half-written this morning.

  1. Yup
  2. Yes, we're only talking about the ImportsModule scanning for the formatters, Dummy creation, and IFakeConfigurator (which I still don't understand). The exception I see is FileLoadException. I did not see anything else on it that would help. If, however, we are okay with stdout as our logger of choice as @patrik-hagne suggests (brilliant! I keep forgetting that we're not a "production" library, so why not log to console?), I'm not against this option. Failing silently frightens me.
  3. Yes and no. In some respects it's a weaker version of number 2, but it has its own charm. Specifically, by skipping assemblies with the same filename, we'd have faster startup when shadow copies are turned on. Still, I'm not excited about throwing that change in hastily just to work around the bug we have now.
  4. Yup, we'd need affected people to do a thing. That we don't have a syntax for. Again, I'm not entirely against it, it would give people the power to speed up/bypass assembly scanning if they wanted to. Still, I wouldn't want to knee-jerk this in just for the bug.

So, number 2 could be okay. I worry that people who get hit by this all the may be annoyed by the output, but I guess they'd have the option to workaround.

Do we want to ship a 1.14.1 out quickly? I think I can find time to work on the fix later tonight/tomorrow morning. (Daylight savings ends tonight. Tomorrow has 25 hours!)

Now we can wrangle over the error message! Oh, and unit/integration testing could be a pain. Hmm…

@adamralph
Copy link
Contributor

Assuming this problem has been present for some time and given there is a workaround, I don't think there's any rush to hurry this out.

As for the error message, I guess something straightforward like "Warning: Failed to load assembly 'C:\foo\bar.dll'" followed by the exception message.

I imagine a unit test won't be possible for this fix. Perhaps there's a way to cobble together an integration/acceptance test though.

@blairconrad
Copy link
Member

Yeah, I'll try the cobbling. Not so sure about the message. I think that would leave unanswered questions. Then again, something reassuring may be too long. You may have noticed, but have a tendency to ramble on.

Warning: FakeItEasy failed to load assembly 'C:\foo\bar.dll' while scanning for extension points. Any IArgumentValueFormatters, IDummyDefinitions, and IFakeConfigurators in that assembly will not be available."

Then followed by the exception message?

@adamralph
Copy link
Contributor

I like it! Much better.

I presume we don't have to include the stacktrace? I guess people can debug their tests if they need to go down to that level.

@blairconrad
Copy link
Member

I think stack trace will add little or nothing... especially since the catch will be at a known place. And any FakeItEasy call could trigger the scan. I am inclined to skip it.

@adamralph
Copy link
Contributor

Agreed.

@patrik-hagne
Copy link
Member

IFakeConfigurator has to do with the feature descrdibed here: http://ondevelopment.blogspot.se/2009/11/configuring-fake-objects-on-global.html

@blairconrad
Copy link
Member

Thanks. That'll help with #143, when I get back to it…

@blairconrad
Copy link
Member

@alexjamesbrown, I'm still working on a nicely-tested fix for this, but I do have an ad hoc FakeItEasy that contains what I expect will be very close to the fix.

If you have time and interest, would you grab it and see if it makes your tests pass, even when run the old way?

Thanks!

@alexjamesbrown
Copy link
Author

Yeah, sounds good...
Where do i get it? Will give this a go when I get into the office tomorrow?

@blairconrad
Copy link
Member

It's in my dropbox. I put a link in the last comment. Let's see if this works better:

https://www.dropbox.com/sh/u11tvi69s74jhe6/MAfbHD7x61

I don't often share this way. Tell me if you can't get and we'll make other arrangements.

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