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

Executables for multi-node testing and first multi-node cluster test #497

Merged
merged 1 commit into from
Nov 23, 2014
Merged

Executables for multi-node testing and first multi-node cluster test #497

merged 1 commit into from
Nov 23, 2014

Conversation

smalldave
Copy link
Contributor

Here are the beginnings of what is needed for the cluster integration tests.
This adds two new executables
Akka.MultiNodeTestRunner and Akka.NodeTestRunner (better names appreciated).

Both executables use xunit 2.0 to do unit test discovery and execution.
Multi-node tests will have to be written using xunit.

MultiNodeTestRunner
Looks at the test assembly specified as a command line argument and picks up all the relevant tests that it should run (currently all tests that end in a number, should be a bit more sophisticated).

A multi-node test looks like this

public class InitialHeartbeatMultiNode1 : InitialHeartbeatSpec
{
}

public class InitialHeartbeatMultiNode2 : InitialHeartbeatSpec
{
}

public class InitialHeartbeatMultiNode3 : InitialHeartbeatSpec
{
}

public abstract class InitialHeartbeatSpec : MultiNodeClusterSpec

The MultiNodeTestRunner looks at this, works out that it needs to create 3 processes to run 3 nodes for the test.
It executes NodeTestRunner in each process to do this passing parameters on the command line

NodeTestRunner
Picks up the parameters passed to it and executes the test specified.

I've implemented JoinInProgressSpec. It all seems to work with nodes waiting to enter barriers correctly.
There are also a number of fixes for Remote.TestKit in here.

Thoughts much appreciated.

@Aaronontheweb
Copy link
Member

This is awesome Dave - sorry for not responding to your PR on my personal branch. Been tied up doing on another project :\

akka.remote.log-remote-lifecycle-events = off
akka.loggers = [""Akka.Testkit.TestEventListener""]
#akka.remote.log-remote-lifecycle-events = off
#akka.loggers = [""Akka.TestKit.TestEventListener, Akka.TestKit""]
akka.test {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat OT but this change got me thinking. Should we add support for case-insensitive aliases, so for example
"akka.testkit.testeventlistener" would be an alias for the type name
"Akka.TestKit.TestEventListener, Akka.TestKit" ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean built in aliases for core types?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep.
The thing is, with the TestEventListener you have to specify the fullname, i.e. including assembly name, as it's not in the Akka assembly. For types in the Akka assembly you don't. That got me thinking that we should have an alias for TestEventListener, i.e. "Akka.TestKit.TestEventListener" without having to specify the assembly name.
The second part is the casing. I realized that maybe we should ignore casing for the core (and TestKit) types so that "akka.testkit.testeventlistener" and "akka.Testkit.TestEventListener" would be valid.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.
I'd like to see the type resolution abstracted away from Type.GetType to something that forces us to verify the resulting type.
e.g.

//just from the top of my head, but you get the idea
Type ResolveType(string typeNameOrAlias,Action typeNotFound, Type mustBeAssignableTo = null)
{
     //resolve type and call typeNotFound if no such type found..   
     //ensure type is assignable to mustBeAssignableTo 
}

We use Type.GetType all over the code base with none to some verification on the result.

@HCanber
Copy link
Contributor

HCanber commented Nov 12, 2014

@smalldave I glance through the code. This is way outside my comfort zone. :) One thought: As this is hardwired to use xUnit2 that should be reflected in the project name and namespace, maybe Akka.MultiNodeTestRunner.XUnit2 and Akka.NodeTestRunner.XUnit2 so that if someone comes along and creates one for mspec or whatever it can be named accordingly.

@smalldave
Copy link
Contributor Author

The xunit.abstrations library, which is what I'm using for discovery / running tests would allow other frameworks to be used if they implemented the necessary classes. Somebody who wanted to use mspec could take that approach. Not sure that is necessarily what they would want to do though?

@HCanber
Copy link
Contributor

HCanber commented Nov 12, 2014

Yeah, maybe so, but hat doesn't change the fact that everything right now is hardwired to xUnit2, right?

Multi-node tests will have to be written using xunit.

If someone really wanted this for mspec, they'd have to create new stuff (copy-paste your work or something, which is perfectly fine). What would they name those? Wouldn't they be named something along Akka.MultiNodeTestRunner.Mspec & Akka.NodeTestRunner.MSpec ?
Or am I misunderstanding? :)

@smalldave
Copy link
Contributor Author

They could implement the xunit.abstractions classes for mspec. That way we could use the same executables and use configuration to specify which unit test framework to run.
I'm not sure why xunit.abstractions exists. I don't know if the aim is to provide a set of unit test framework abstractions for .net. Quite probably not as the name seems to tie it to one framework.
I'm not against calling them ...Xunit2 just wondering if the abstractions route makes sense?

@rogeralsing
Copy link
Contributor

Agree with @HCanber , this is out of my comfort zone too, but it is very isolated so I think we are safe

rogeralsing added a commit that referenced this pull request Nov 23, 2014
Executables for multi-node testing and first multi-node cluster test
@rogeralsing rogeralsing merged commit 597f633 into akkadotnet:dev Nov 23, 2014
@HCanber
Copy link
Contributor

HCanber commented Nov 23, 2014

My machine and AppVeyor fails on AClusterNodeMustSendHeartbeatsImmediatelyWhenJoiningToAvoidFalseFailureDetectionDueToDelayedGossip. Ping @smalldave

[DEBUG][2014-11-23 19:11:06][Thread 0058][EventStream] StandardOutLogger started
[DEBUG][2014-11-23 19:11:06][Thread 0058][EventStream(MultiNodeClusterSpec)] Logger log66-TestEventListener [TestEventListener] started
[DEBUG][2014-11-23 19:11:06][Thread 0058][EventStream(MultiNodeClusterSpec)] StandardOutLogger being removed
[INFO][2014-11-23 19:11:06][Thread 0058][remoting] Starting remoting
[INFO][2014-11-23 19:11:06][Thread 0058][remoting] Remoting started; listening on addresses : [akka.tcp://MultiNodeClusterSpec:2552]
[INFO][2014-11-23 19:11:06][Thread 0058][remoting] Remoting now listens on addresses: [akka.tcp://MultiNodeClusterSpec:2552]
[INFO][2014-11-23 19:11:06][Thread 0058][Cluster] Cluster Node [akka.tcp://MultiNodeClusterSpec:2552] - Starting up...
[INFO][2014-11-23 19:11:06][Thread 0060][[akka://MultiNodeClusterSpec/system/cluster/core/daemon]] No seed-nodes configured, manual cluster join required

System.ArgumentNullExceptionValue cannot be null.
Parameter name: hostNameOrAddress
   at System.Net.Dns.GetHostEntry(String hostNameOrAddress)
   at Akka.Remote.TestKit.MultiNodeSpec..ctor(RoleName myself, ActorSystem system, ImmutableList`1 roles, Func`2 deployments) in MultiNodeSpec.cs: line 296
   at Akka.Remote.TestKit.MultiNodeSpec..ctor(MultiNodeConfig config) in MultiNodeSpec.cs: line 281
   at Akka.Cluster.Tests.MultiNode.MultiNodeClusterSpec..ctor(MultiNodeConfig config) in MultiNodeClusterSpec.cs: line 127
   at Akka.Cluster.Tests.MultiNode.JoinInProgressSpec..ctor(JoinInProgressMultiNodeSpec spec) in JoinInProgressSpec.cs: line 52
   at Akka.Cluster.Tests.MultiNode.JoinInProgressSpec..ctor() in JoinInProgressSpec.cs: line 48
   at Akka.Cluster.Tests.MultiNode.JoinInProgressMultiNode1..ctor()

@smalldave
Copy link
Contributor Author

Ah. That was something I meant to discuss. As with JVM Akka these tests have to be run from the
special executable. They can't be run by the xunit test runner alone.
I think the JVM version flags the tests as 'LongRunning' and they are ignored for standard unit testing.
Perhaps we should add and ignore a category (trait) for these tests?
These tests will take quite a while to run. From memory the JVM version took a good 10 minutes.
Not sure how we'd want to set that up with the CI server?

@smalldave smalldave deleted the multinodetest branch November 24, 2014 09:53
@HCanber
Copy link
Contributor

HCanber commented Nov 24, 2014

For now, it's probably best if we just removed them from being run (if adding a Trait is enough, do that) and then set up a new target in build.fsx that executes the test.

@smalldave
Copy link
Contributor Author

Ok. Will take a look at that

@HCanber
Copy link
Contributor

HCanber commented Nov 24, 2014

👍

Aaronontheweb added a commit to Aaronontheweb/akka.net that referenced this pull request Dec 13, 2014
__Brand New F# API__. The entire F# API has been updated to give it a
more native F# feel while still holding true to the Erlang / Scala
conventions used in actor systems. [Read more about the F# API
changes](akkadotnet#526).

__Multi-Node TestKit (Alpha)__. Not available yet as a NuGet package,
but the first pass at the Akka.Remote.TestKit is now available from
source, which allow you to test your actor systems running on multiple
machines or processes.

A multi-node test looks like this

public class InitialHeartbeatMultiNode1 : InitialHeartbeatSpec
{
}

public class InitialHeartbeatMultiNode2 : InitialHeartbeatSpec
{
}

public class InitialHeartbeatMultiNode3 : InitialHeartbeatSpec
{
}

public abstract class InitialHeartbeatSpec : MultiNodeClusterSpec
The MultiNodeTestRunner looks at this, works out that it needs to create
3 processes to run 3 nodes for the test.
It executes NodeTestRunner in each process to do this passing parameters
on the command line. [Read more about the multi-node testkit
here](akkadotnet#497).

__Breaking Change to the internal api: The `Next` property on
`IAtomicCounter<T>` has been changed into the function `Next()`__ This
was done as it had side effects, i.e. the value was increased when the
getter was called. This makes it very hard to debug as the debugger kept
calling the property and causing the value to be increased.

__Akka.Serilog__ `SerilogLogMessageFormatter` has been moved to the
namespace `Akka.Logger.Serilog` (it used to be in
`Akka.Serilog.Event.Serilog`).
Update your `using` statements from `using Akka.Serilog.Event.Serilog;`
to `using Akka.Logger.Serilog;`.

__Breaking Change to the internal api: Changed signatures in the
abstract class `SupervisorStrategy`__. The following methods has new
signatures: `HandleFailure`, `ProcessFailure`. If you've inherited from
`SupervisorStrategy`, `OneForOneStrategy` or `AllForOneStrategy` and
overriden the aforementioned methods you need to update their
signatures.

__TestProbe can be implicitly casted to ActorRef__. New feature. Tests
requring the `ActorRef` of a `TestProbe` can now be simplified:
``` C#
var probe = CreateTestProbe();
var sut = ActorOf<GreeterActor>();
sut.Tell("Akka", probe); // previously probe.Ref was required
probe.ExpectMsg("Hi Akka!");
```

__Bugfix for ConsistentHashableEvenlope__. When using
`ConsistentHashableEvenlope` in conjunction with
`ConsistentHashRouter`s, `ConsistentHashableEvenlope` now correctly
extracts its inner message instead of sending the entire
`ConsistentHashableEvenlope` directly to the intended routee.

__Akka.Cluster group routers now work as expected__. New update of
Akka.Cluster - group routers now work as expected on cluster
deployments. Still working on pool routers. [Read more about
Akka.Cluster routers
here](akkadotnet#489).
This was referenced Dec 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants