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

NUnit tests fail if the fixture contains more than one test #1065

Closed
garethbudden opened this issue Jun 18, 2015 · 12 comments
Closed

NUnit tests fail if the fixture contains more than one test #1065

garethbudden opened this issue Jun 18, 2015 · 12 comments

Comments

@garethbudden
Copy link
Contributor

I've hit a problem when trying to use the NUnit test suite, given the following example...

public class NUnitTestFixture
{
    [Test]
    public void SayHello()
    {
        ActorOf<Parrot>("test").Tell(new SayHello());

        ExpectMsg<Hello>();
    }

    [Test]
    public void SayGoodbye
    {
        ActorOf<Parrot>("test").Tell(new SayGoodbye());

        ExpectMsg<Goodbye>();
    }
}

When I run the tests the first succeeds but the second fails with Akka.Actor.InvalidActorNameException : Actor name "test" is not unique!

If I instead run the same tests but with XUnit then they succeed, I believe this could be related to #972, which says that NUnit keeps the ActorSystem around for the full test fixture, but XUnit creates a new ActorSystem for each test.

@rogeralsing
Copy link
Contributor

This is definitely a bug in the NUnit TestKit, this package is pretty new and thus not very well tested in the field.
We use XUnit TestKit internally.

@garethbudden
Copy link
Contributor Author

One option would be to wipe the ActorSystem after each test run, as was mentioned in #972 this may not be performant, on the other hand it would be more consistent with how XUnit works. Or is there a better way to handle this entirely?

@rogeralsing
Copy link
Contributor

Yes, imo, we should spin up a new instance of the system and init all the defaults after each test.
This is after all what happens in the XUnit Testkit, each parallel test has its own system and infrastructure

@Aaronontheweb
Copy link
Member

Is this really a bug? The current design seems right to me - the actor system should be shared across all of the tests in a fixture. Our multi-node tests would all break if this wasn't the case.

@Aaronontheweb
Copy link
Member

Ah, I take that back about the multi-node tests - that's not the case there.

@rogeralsing
Copy link
Contributor

If the system was shared between tests, we would end up with a gazillion racy tests with completely undeterministic behavior

@Aaronontheweb
Copy link
Member

Where inside the XUnit testkit does the actor system get shut down after each test currently?

(edit: I tried looking but it isn't obvious to me)

@rogeralsing
Copy link
Contributor

Yes, there is one instance per test in XUnit, all isolated

@Aaronontheweb
Copy link
Member

Yes, there is one instance per test in XUnit, all isolated

Can you walk me through that inside the code of the XUnit fixture itself? Like what's the call chain that actually makes this happen per-test? Everything looks per-fixture to me.

@rogeralsing
Copy link
Contributor

        [Fact]
        public void Unique1()
        {
            var hash= Sys.GetHashCode();
            Console.WriteLine(hash);
        }

        [Fact]
        public void Unique2()
        {
            var hash = Sys.GetHashCode();
            Console.WriteLine(hash);
        }

Run that in a testkit test and you will see that the systems in the two tests get different hash codes.
XUnit does weird scheduling so you cant actually step from the test init to the test methods

Or, just set a BP in the test's ctor and you will see that its called as many times as there are tests running

@Aaronontheweb
Copy link
Member

Ok, got it. In that case I guess we should add a cleanup attribute to a method inside the NUnit testkit?

@garethbudden
Copy link
Contributor Author

I thought I may take a stab at this one.

I've had a quick look and I think the simplest thing to do may be to take all the private fields from TestKitBase.cs and put them into a new private class called TestKitBaseState.

The XUnit implementation would be able to create and dispose this class using the constructor and dispose method, as it does now. The NUnit implementation would be able to create a new TestKitBaseState instance per unit test using the SetUp attribute, and dispose of the state class using the TearDown attribute.

I've not used MsTest myself but a quick google scan implies the same issue as NUnit, and could be fixed with the same approach.

Before I make an attempt at implementing it I thought I'd check if this sound reasonable to you?

garethbudden added a commit to garethbudden/akka.net that referenced this issue Jun 24, 2015
garethbudden added a commit to garethbudden/akka.net that referenced this issue Jun 25, 2015
Aaronontheweb added a commit that referenced this issue Jun 26, 2015
…fter-each-test-run

Fix for issue #1065 NUnit to destroy ActorSystem after each test run inline with XUnit
Horusiath pushed a commit to Horusiath/akka.net that referenced this issue Jul 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants