Lagom: PersistentEntityTestDriver improvements

[ignasi35]

Tests based on PersistentEntityTestDriver present some repetitive idioms like:

• asserting there's no serialization issues
• asserting every command handler sends a reply

Writing the tests in Java using JUnit there's a common snippet in an @After method.

Writing tests in Scala (using scalatest) has similar code duplication. In scalatest-based tests in particular Lagom's testkit could provide ScalaTest fixture traits to remove the duplication and provide default assertions.

Credit: inspired by scalatestplus-play

[erip]

Obligatory background from associated PR:

[...] The problem is that often, the driver is used more than once on a single test: send a couple of commands to set the proper state before actually sending the command under test. In that case, if the serialization issue was on a command that is not the last one and allIssues is reset on every run invocation, then the test would not fail. [...] There's a completely different approach to solve the idiom duplication which is to create a FailFastPersistentEntityTestDriver which, you guessed it, fails when there's issues without the user having to run the assertion. I think that'd could be done on a separate PR.

The driver resets the issues on each invocation (java and scala) -- I think this might be considered a bug given that it isn't actually true that the issues returned by getAllIssues are _all_ of the issues over the lifetime of a driver.

We could fix this pretty easily, I think... Not sure if there are downstream implications... WDYT, @ignasi35? By fixing this, we could ostensibly fail fast (or allow a client to opt-in to fail fast).

[ignasi35]

We could fix this pretty easily, I think... Not sure if there are downstream implications... WDYT, @ignasi35? By fixing this, we could ostensibly fail fast (or allow a client to opt-in to fail fast).

I'm in favour of failing fast.

Changing the behavior or run so that it always asserts the existence of issues before completing and causes the test to fail (using test library tools or throwing an exception) sounds good to me. The implications are that teams may start to see their tests failing (which I think is a good thing in all cases).

Note: there are several types of issues:

• serialization
• unhandled command
• unhandled event

---

The driver resets the issues on each invocation (java and scala)

Upon closer inspection, the driver doesn't reset allIssues on every invocation to run(); it resets only issues. See how the driver has two fields and accumulates issues into allIssues.

---

[erip]

I can come up with a fix and submit it as a part of the linked PR (since it's strictly a testkit improvement.😅)... I have some ideas.

[PerWiklander]

You are probably already on top of this, but I thought I might contribute the trait that my entity tests extend:

import akka.actor.ActorSystem
import akka.testkit.TestKit
import com.lightbend.lagom.scaladsl.persistence.PersistentEntity
import com.lightbend.lagom.scaladsl.playjson.JsonSerializerRegistry
import com.lightbend.lagom.scaladsl.testkit.PersistentEntityTestDriver
import org.scalatest.{BeforeAndAfterAll, Matchers, WordSpec}

trait EntitySpec[
  C,
  E,
  S,
  Entity <: PersistentEntity { type Command = C; type Event = E; type State = S}
] extends WordSpec
  with Matchers
  with BeforeAndAfterAll
{
  protected def entity:             Entity
  protected def entityId:           String
  protected def serializerRegistry: JsonSerializerRegistry

  protected val system = ActorSystem(
    this.getClass.getSimpleName,
    JsonSerializerRegistry.actorSystemSetupFor(serializerRegistry)
  )

  override protected def afterAll(): Unit = {
    TestKit.shutdownActorSystem(system)
  }

  protected def withTestDriver(block: PersistentEntityTestDriver[C, E, S] => Unit): Unit = {
    val driver = new PersistentEntityTestDriver(system, entity, entityId)

    block(driver)

    driver.getAllIssues should have size 0
  }
}

And a test:

class DataSetEntitySpec
  extends EntitySpec[
    DataSetCommand[_],
    DataSetEvent,
    DataSetState,
    DataSetEntity
] {

  override protected def entity             = new DataSetEntity
  override protected def entityId           = "DataSet_1"
  override protected def serializerRegistry = DataSetSerializerRegistry

  "The data set entity" should {
    ...
  }
}

[erip]

@PerWiklander There's an open PR; the issue is that we don't want to marry this to scalatest, so we can't depend on those mixins. See the linked PR.

[PerWiklander]

Sorry, didn't check the PR first. Looks like your implementation is very much like mine except throwing an exception instead of using test framework specific checks. And nothing is stopping a user from extending your trait into something more test framework specific if one wanted to do so.

[erip]

@PerWiklander Right. My initial implementation was almost exactly yours, but since we don't want to be too prescriptive with testing libs (some people use specs2??? 😏), we thought throwing might be a more universal "stop testing" signal (which I don't particularly like).