Improvements regarding system shutdown and usage of JavaTestKit

Question

Improvements regarding system shutdown and usage of JavaTestKit

eshepelyuk opened this issue 8 years ago · 15 comments

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

Answer 1 · 2016-09-20T05:12:20.000Z

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

Sondre

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
.

Answer 2 · 2016-09-20T05:21:16.000Z

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

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.
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 ?

Answer 3 · 2016-09-20T05:36:59.000Z

Sounds like a good idea!
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

Answer 4 · 2016-09-20T06:21:54.000Z

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.

Answer 5 · 2016-09-20T06:56:37.000Z

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?

Answer 6 · 2016-09-20T07:06:09.000Z

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

Answer 7 · 2016-09-20T19:23:19.000Z

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.

Answer 8 · 2016-09-20T20:44:23.000Z

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

Answer 9 · 2016-09-21T06:46:33.000Z

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 ?

Answer 10 · 2016-09-21T06:53:31.000Z

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

Answer 11 · 2016-09-21T06:54:58.000Z

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

Answer 12 · 2016-09-22T20:04:46.000Z

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

Answer 13 · 2016-09-23T07:16:17.000Z

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 ?

Answer 14 · 2016-09-23T07:45:56.000Z

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
.

Answer 15 · 2016-09-23T08:40:23.000Z

Created Issue #5