Improvements regarding system shutdown and usage of JavaTestKit

[eshepelyuk]

Hello @zapodot
What is the Currently state of the project ? Are you still maintaining it ?
I have an idea for some improvement and PR

[zapodot]

Hi! All suggestions and PR's are more than welcome ;-)

Sondre

1. sep. 2016 6.39 a.m. skrev "Evgeny Shepelyuk" notifications@github.com:

Hello @zapodot https://github.com/zapodot
What is the Currently state of the project ? Are you still maintaining it ?
I have an idea for some improvement and PR

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAhqkSJcfSuiglKMX2DhMQeJPaiT88Ruks5qr2NtgaJpZM4KBN45
.

[eshepelyuk]

Great !
So, basically, I have two proposals for now

1. Use JavaTestKit#shutdown when stopping actor system, instead of ActorSystem#shutdown. The first method allows to wait until system completely stopped, thus helping to avoid various race conditions that can happens between rests.
2. For the rule I'd like to introduce a method for obtaining instance of JavaTestKit, I.e new JavaTestKit(system). It's a common pattern in our projects to instantiate ActorSystem and then corresponding JavaTestKit, so that method could reduce code duplication in tests.

What do you think ?

[zapodot]

1. Sounds like a good idea!
2. No problem 👍 Only thing to consider: The JavaTestKit created needs to be shutdown cleanly. Either should a reference to the given TestKit be kept for reference and cleaned up after the tests has completed OR we have to leave this responsibility to the developer of the test

[eshepelyuk]

Only thing to consider: The JavaTestKit created needs to be shutdown cleanly. Either should a reference to the given TestKit be kept for reference and cleaned up after the tests has completed OR we have to leave this responsibility to the developer of the test

Well, JavaTestKit#shutdown just calls static method JavaTestKit#shutdownActorSystem.
So, implementing point 1 will solve the issue.

I'll try to submit PR this weekend.

[zapodot]

Another aproach will be to extend the existing @rule and create a JavaTestKitRule that takes care of both startup and shutdown. The drawback is that you can have only one JavaTestKit instance per test. Would that cause any problems for you @eshepelyuk?

[eshepelyuk]

I don't see any problem, just not clearly understand the benefits of creating a new @Rule ?
Can you provide more details ?

[zapodot]

I do think that a JavaTestKitRule will cover a lot of use cases as I suspect that many people need to create a JavaTestKit anyway. Having all the logic contained in a rule ensures that all resources are freed after the test has completed and also avoids code duplication in people's tests.

[zapodot]

I have added a new TestKitRule and a test class to show how it may be used. It would be awesome if you would have a look at TestKitRuleTest and see if it indeed serves your use-cases. If you are satisfied I will create a new release ASAP

[eshepelyuk]

Hello

The code looks great, doing exactly what we've been missing.
I just still have unanswered question about what are the benefits of having two separate @Rule, i.e.

• ActorSystemRule
• TestKitRule

Don't you think it's just rather confusing to pick up which one to choose ?

[zapodot]

Ok: that's a valid point. They are already almost identical. I'll keep the ActorSystemRule move the JavaTestKit functionality over there. I will look into it at some point in the comming days..

Anyway: thank you for your input

[eshepelyuk]

Thanks for your work on this, waiting forward for the release :)

[zapodot]

As I just released v. 1.2.0 that contains the changes discussed in this ticket, I am closing it 👍

[eshepelyuk]

Unfortunately during shutdown it's not using verifySystemShutdown = true parameter in akka.testkit.JavaTestKit#shutdownActorSystem(akka.actor.ActorSystem, java.lang.Boolean)

This is usually a main cause of various bugs. Could you please add it ?

[zapodot]

Sure!

Could you open a new github issue for that particular bug. It makes it
easier to seperate it from other issues.

On Fri, Sep 23, 2016 at 9:16 AM, Evgeny Shepelyuk notifications@github.com
wrote:

Unfortunately during shutdown it's not using verifySystemShutdown = true
parameter in akka.testkit.JavaTestKit#shutdownActorSystem(akka.actor.ActorSystem,
java.lang.Boolean)

This is usually a main cause of various bugs. Could you please add it ?

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAhqkejxfHLjak4dxcRII6vrZwK31Beqks5qs3zBgaJpZM4KBN45
.

[eshepelyuk]

Created Issue #5