My initial impressions of akka.typed design.

[Sean Shubin]

ActorRef has a self type of ActorRefImpl.  This means that if I want to fake/stub/mock an ActorRef for test purposes, I cannot because I am forced to know about ActorRefImpl.  Instead of users of ActorRef being forced to only use the ActorRefImpl implementation, ActorRef should be a minimal interface that only exposes the essential characteristics of an ActorRef, freeing up users to chose among implementations of ActorRef, including fakes/stubs/mocks for testing, and an ActorRefImpl for production.  As it is designed now, I don't see how one can easily test akka.typed code without specialized libraries that know about ActorRefImpl.

I find that hard to test code is usually a result of design decisions rather than the inherent nature of the problem, and with careful attention and imagination one can make code easy to test without having to use a specialized library.

I am just starting to learn akka typed, so hopefully I will have time to give more detailed feedback than just the one example once I am more familiar with the design.  I am hoping that before removing the warning about how the akka.typed API and semantics may change, the design is refactored to support a more test driven, design by contract approach.  I love the idea of having a statically typed, event driven option for how I code, so am really glad akka.typed is being worked on.

[Roland Kuhn]

Hi Sean,

thanks for discussing Akka Typed!

The design decision behind splitting the interface into a public and a private part is documented in the ScalaDoc for ActorRefImpl: end users shall not ever see the top two methods because there cannot possibly be a reason to use them—but the implementation will need them. Therefore I felt that internals are leaked by having just one interface. Another (minor) issue is that equality and toString are fixed by the semantics of the interface and the self-type ensures that this contract is not broken.

Thankfully, the whole design of Akka Typed is much different from untyped Akka, where this kind of separation was forced by implementation details. In Akka Typed we are free to fold ActorRefImpl into ActorRef and revert the decision. We should then make ActorRef an abstract class, so that it can also safely be used from Java without losing the semantic contract. The downside would remain: exposing methods that don’t belong into user space.

May I ask in how far your intended ActorRef implementation would be different from TestProbe or Inbox? Since there is only a single purpose to ActorRef—sending single messages—we should be able to include all needed stub functionality in one implementation; my guess is that your fake/stub/mock would reimplement what is already there but I may well be missing something.

Another point that I’d like to hear more about is how you intend to test your behaviors in general. This question stems from some surprise at hearing “hard to test” in conjunction with Akka Typed :-) (given that testability is an explicit design goal)

Regards,

Roland

--
>>>>>>>>>> Read the docs: http://akka.io/docs/
>>>>>>>>>> Check the FAQ: http://doc.akka.io/docs/akka/current/additional/faq.html
>>>>>>>>>> Search the archives: https://groups.google.com/group/akka-user
---
You received this message because you are subscribed to the Google Groups "Akka User List" group.
To unsubscribe from this group and stop receiving emails from it, send an email to akka-user+...@googlegroups.com.
To post to this group, send email to akka...@googlegroups.com.
Visit this group at https://groups.google.com/group/akka-user.
For more options, visit https://groups.google.com/d/optout.

[Sean Shubin]

I will take a look at the ScalaDoc for ActorRefImpl.

To answer your question about how I intend to test behaviors, let me clarify that my concern was not about testing behaviors specifically.

I was starting out by testing my interactions with behaviors.

Hopefully I can illustrate what I mean with a couple of examples below.

As you can see in FooApp, it is trivially easy for me to test interactions with an ActorRef if it does not have a self type constraint.

As you can see in BarApp, the self-type forces me to add another level of indirection in the form of ActorRefContract and ActorRefDelegate.

My initial impression is that this is unnecessary complexity that could have been designed away by a "design by contract" approach.
Alternatively, ActorRef could be made to extend another trait without a self type, and all interactions with actors could be through that trait instead of through an ActorRef.

Hopefully this clarifies where my initial design concern is.

import scala.collection.mutable.ArrayBuffer

object FooApp extends App {

  trait ActorRef[T] {
    def !(msg: String): Unit
  }

  class Coordinator(greeter: ActorRef[String]) {
    def newcomer(name: String): Unit = {
      greeter ! name
    }
  }

  class ActorRefStub extends ActorRef[String] {
    val messagesSent = new ArrayBuffer[String]()

    override def !(msg: String): Unit = messagesSent.append(msg)
  }

  def assertEquals[T](caption: String, actual: T, expected: T): Unit = {
    if (actual == expected) println(s"SUCCESS($caption)")
    else println(s"FAILURE($caption): expected $expected, got $actual")
  }

  // given
  val actorRefStub = new ActorRefStub()
  val coordinator = new Coordinator(actorRefStub)

  // when
  coordinator.newcomer("world")

  // then
  assertEquals("exactly one message sent", actorRefStub.messagesSent.size, 1)
  assertEquals("the right message was sent", actorRefStub.messagesSent(0), "world")

  /* output
  SUCCESS(exactly one message sent)
  SUCCESS(the right message was sent)
   */
}

import akka.typed.ActorRef

import scala.collection.mutable.ArrayBuffer

object BarApp extends App {

  trait ActorRefContract[T] {
    def !(msg: T): Unit
  }

  class ActorRefDelegate[T](delegateToMe: ActorRef[T]) extends ActorRefContract[T] {
    override def !(msg: T): Unit = delegateToMe ! msg
  }

  class Coordinator(greeter: ActorRefContract[String]) {
    def newcomer(name: String): Unit = {
      greeter ! name
    }
  }

  class ActorRefStub extends ActorRefContract[String] {
    val messagesSent = new ArrayBuffer[String]()

    override def !(msg: String): Unit = messagesSent.append(msg)
  }

  def assertEquals[T](caption: String, actual: T, expected: T): Unit = {
    if (actual == expected) println(s"SUCCESS($caption)")
    else println(s"FAILURE($caption): expected $expected, got $actual")
  }

  // given
  val actorRefStub = new ActorRefStub()
  val coordinator = new Coordinator(actorRefStub)

  // when
  coordinator.newcomer("world")

  // then
  assertEquals("exactly one message sent", actorRefStub.messagesSent.size, 1)
  assertEquals("the right message was sent", actorRefStub.messagesSent(0), "world")

  /* output
  SUCCESS(exactly one message sent)
  SUCCESS(the right message was sent)
   */
}

[Sean Shubin]

I had a chance to look over the typed actors a bit more.  Here is my feedback.  Whether you agree or disagree, I hope you find it useful.  I am still really excited about typed actors, the main reason I have been reluctant to switch to akka is that I was not willing to give up static typing.  

Behavior by itself is easy to test as long as you are not doing anything with ActorContext.  I particularly like the separation of the management method to deal with lifecycle signals, from the message method to deal with messages.

ActorContext can't be stubbed out because it leaks out the private classes InternalActorRef and ActorSystemImpl through its protected methods lookupRoot, guardian, and systemImpl.

ActorRef can't be stubbed out because of the self type on internal.ActorRefImpl.  I have already demonstrated issue with this in detail with my FooApp and BarApp examples in this thread.  I did look up the ScalaDoc for ActorRefImpl, and while I think its fine to hide the sendSystem and isLocal methods, I do think that if these methods were pushed down into a class that inherits from ActorRef, rather than using a self type, that would make it easier to stub out ActorRef.

ActorSystem has the same problem as ActorRef for the same reason, but it manifests much worse because the ActorSystem trait has so many more methods.  To see why, consider writing a unit test to make sure an actor system is properly shut down once the program is finished.  I want to test the code here:

https://github.com/SeanShubin/concurrency-samples/blob/master/actor/src/main/scala/com/seanshubin/concurrency/samples/actor/CleanupActorSystems.scala

It should be as easy as this:

https://github.com/SeanShubin/concurrency-samples/blob/master/actor/src/test/scala/com/seanshubin/concurrency/samples/actor/CleanupActorSystemsTest.scala

And from looking at this test it seems like it is, but look what I had to do to get to this point:

On my dependency injection class I had to add another level of indirection

  val eventActorSystem: ActorSystem[Event] = ActorSystem("state", stateful)

  val eventActorSystemContract: ActorSystemContract[Event] = 

    new ActorSystemDelegate[Event](eventActorSystem)

I had to hide ActorSystem behind an interface

https://github.com/SeanShubin/concurrency-samples/blob/master/actor/src/main/scala/com/seanshubin/concurrency/samples/actor/ActorSystemContract.scala

And then delegate to the real ActorSystem

https://github.com/SeanShubin/concurrency-samples/blob/master/actor/src/main/scala/com/seanshubin/concurrency/samples/actor/ActorSystemDelegate.scala

That way, for testing I could start with a class where nothing is implemented:

https://github.com/SeanShubin/concurrency-samples/blob/master/actor/src/test/scala/com/seanshubin/concurrency/samples/actor/ActorSystemNotImplemented.scala

Then use that not implemented class as a base for my stub

  class ActorSystemStub extends ActorSystemNotImplemented[Event] {

    var terminateInvocationCount = 0

    override def terminate(): Future[Terminated] = {

      terminateInvocationCount += 1

      null

    }

  }

Which I finally use in my test

https://github.com/SeanShubin/concurrency-samples/blob/master/actor/src/test/scala/com/seanshubin/concurrency/samples/actor/CleanupActorSystemsTest.scala

My impression is that most of these problems stem from exposing classes to the user that are too concrete.  If we could push some of these implementation details like private clases and self types further down the inheritance chain, and remove the need for client code to have a compile time dependency on the implementation details, it would be much easier to stub out interactions and have fully unit-tested code.

[Roland Kuhn]

Hi Sean,

removing the self-type would not help: its function is only to ensure that every ActorRef also has an appropriate implementation, basically proving that the down-cast that is used in many places in the implementation will work out fine. In the end every ActorRef needs to support the sendSystem method (not equally sure about isLocal, due to missing remoting implementation).

Patrik & team: what is your opinion on this? Should sendSystem become visible in order to make ActorRef implementable by user code? In any case we should probably prominently point out that suitable stubs for ActorRef and ActorContext are provided in the form of Inbox and EffectfulActorContext, and that rolling your own is typically not necessary.

On stubbing out ActorSystem or making it easier to override, I have mixed feelings: it is a quite complicated machine that is not easy to modify correctly—and it is also not easy to provide a stub that actually works, due to some semantic requirements. I completely understand that just counting calls to terminate() may seem to contradict my statement, but consider the other side of the coin: if ActorSystem were as you want it, then people would modify it also in cases where that is not appropriate.

To answer your concrete question, a much easier way of ensuring correct shutdown would be to supply a function of type () => Unit to the actor that shall call it. Production code would supply () => system.terminate(), but the test could provide whatever you want.

Regards,

Roland

[Patrik Nordwall]

On Sat, Aug 26, 2017 at 8:10 PM, Roland Kuhn <goo...@rkuhn.info> wrote:

Hi Sean,

removing the self-type would not help: its function is only to ensure that every ActorRef also has an appropriate implementation, basically proving that the down-cast that is used in many places in the implementation will work out fine. In the end every ActorRef needs to support the sendSystem method (not equally sure about isLocal, due to missing remoting implementation).

Patrik & team: what is your opinion on this? Should sendSystem become visible in order to make ActorRef implementable by user code?

That would mean that SystemMessage would also be public API. How far would we go? Would the system message implementations also be public?

An alternative would be to provide an ActorRefStub in akka-typed-testkit:

abstract class ActorRefStub extends ActorRefImpl {

  private[typed] final def sendSystem(signal: SystemMessage): Unit = 

    () // silently ignore

}

It would not support things like watch. When would one want to use that instead of Inbox.ref? 

btw, sendSystem is a horrible name, should be sendSignal or sendSystemMessage

 

To unsubscribe from this group and stop receiving emails from it, send an email to akka-user+unsubscribe@googlegroups.com.

To post to this group, send email to akka...@googlegroups.com.
Visit this group at https://groups.google.com/group/akka-user.
For more options, visit https://groups.google.com/d/optout.

--
>>>>>>>>>> Read the docs: http://akka.io/docs/
>>>>>>>>>> Check the FAQ: http://doc.akka.io/docs/akka/current/additional/faq.html
>>>>>>>>>> Search the archives: https://groups.google.com/group/akka-user
---
You received this message because you are subscribed to the Google Groups "Akka User List" group.

To unsubscribe from this group and stop receiving emails from it, send an email to akka-user+unsubscribe@googlegroups.com.

To post to this group, send email to akka...@googlegroups.com.
Visit this group at https://groups.google.com/group/akka-user.
For more options, visit https://groups.google.com/d/optout.

--

Patrik Nordwall
Akka Tech Lead
Lightbend -  Reactive apps on the JVM
Twitter: @patriknw

[Sean Shubin]

My first impulse is to see if the need for down casts can be designed away.  This would remove the contention between trying to enforce certain implementation details while simultaneously hiding those same implementation details from the user.  However I am not familiar enough with the design yet to determine what I would be trading for that.

As for making SystemMessage part of the public api, my first impression was to bury as many implementation details as possible, but after looking at the code, I am only seeing 6 system messages extending a sealed class.  Perhaps going that far would not be so bad if that is the only way to make it easy to stub out any part of the public api.

In my ideal fantasy world, which I understand is not necessarily practical, the entirety of classes I have to deal with are traits with no implementations and no self types, perhaps some factories to choose appropriate implementations for me.  If I want to stay out of trouble, I stick with the factory provided implementations.  If I want to protect myself form crazy implementations of others, I don't use them, or I push for enforcing coding standards that ensure only people who really know what they are doing are mucking about with custom actor systems.

To unsubscribe from this group and stop receiving emails from it, send an email to akka-user+...@googlegroups.com.

To post to this group, send email to akka...@googlegroups.com.
Visit this group at https://groups.google.com/group/akka-user.
For more options, visit https://groups.google.com/d/optout.

--
>>>>>>>>>> Read the docs: http://akka.io/docs/
>>>>>>>>>> Check the FAQ: http://doc.akka.io/docs/akka/current/additional/faq.html
>>>>>>>>>> Search the archives: https://groups.google.com/group/akka-user
---
You received this message because you are subscribed to the Google Groups "Akka User List" group.

To unsubscribe from this group and stop receiving emails from it, send an email to akka-user+...@googlegroups.com.

To post to this group, send email to akka...@googlegroups.com.
Visit this group at https://groups.google.com/group/akka-user.
For more options, visit https://groups.google.com/d/optout.