testcontainers-scala icon indicating copy to clipboard operation
testcontainers-scala copied to clipboard

Easier Usage With Play / Other Servers

Open jamesward opened this issue 4 years ago • 8 comments

When I use this project with servers that need configuration from a started testcontainer, it is a little tricky to setup the starting, configuring, stopping of the container along with the server. Here is what I came up with in the case of Play & ScalaTest:

class FooSpec extends PlaySpec with GuiceOneAppPerSuite {

  override def fakeApplication(): Application = {
    val memcached = GenericContainer("memcached", Seq(11211))
    memcached.start()

    val app = new GuiceApplicationBuilder()
      .configure("memcached.host" -> (memcached.host + ":" + memcached.mappedPort(11211)))
      .build()

    app.injector.instanceOf[ApplicationLifecycle].addStopHook { () =>
      Future.fromTry(Try(memcached.stop()))
    }

    app
  }

}

I.e. I couldn't find a way to combine GuiceOneAppPerSuite with TestContainerForAll or ForAllTestContainer.

jamesward avatar Nov 25 '20 21:11 jamesward

Hi @jamesward, I don't have experience with Play. Would you consider contributing a module that covers your use case?

dimafeng avatar Nov 26 '20 15:11 dimafeng

Yeah, I'll try to take a stab at that.

jamesward avatar Nov 26 '20 16:11 jamesward

I agree, the new API syntax makes it very awkward to use TestContainersForAll with a service that is also used by all tests. This is because of the requirement to now use withContainers on each test block to access the containers. With the old ForAllTestContainers, we could just use container.start() and then reference the container anywhere after (for example, in application configuration).

A typical test looks like:

class MyApplicationTests extends AnyWordSpec {
  val server = // ... start the server, which will need to reference the containers for configuration

  "my test" should {
    "do something with the server" in {
      // ...
    }
  }

solarmosaic-kflorence avatar May 13 '21 23:05 solarmosaic-kflorence

This is because of the requirement to now use withContainers on each test block to access the containers.

This is the one of the recommended by the scalatest docs way of sharing fixtures. Nothing new here from the testcontainers side. withContainers is needed because testcontainers manage the lifecycle of the containers. You can find more information about this in the scalatest docs.

About described use case: you can use afterContainersStart to achieve needed behavior:

// Some service I want to test. For example it requires PostgreSQLContainer and ElasticsearchContainer.
class MyService(pg: PostgreSQLContainer, es: ElasticsearchContainer)

class MyServiceTests extends FreeSpec with TestContainersForAll {

  override type Containers = PostgreSQLContainer and ElasticsearchContainer

  override def startContainers(): Containers = {
    val container1 = PostgreSQLContainer.Def().start()
    val container2 = ElasticsearchContainer.Def().start()
    container1 and container2
  }

  // Here is my service to test. It is uninitialized initially.
  private var myService: MyService = _

  // Here is the place when you can create your service and inject created containers.
  override def afterContainersStart(containers: Containers): Unit = {
    super.afterContainersStart(containers)
    val pg and es = containers
    myService = new MyService(pg, es)
  }

  // Here is he helper method for providing fixtures inside test's bodies.
  def withService[A](runTest: (MyService, Containers) => A): A = withContainers { containers: Containers =>
    runTest(myService, containers)
  }

  "it" - {
    "should work" in withService { case (service, pg and es) =>
      // test code here
    }
  }
}

You can also hide all complexity in a reusable trait:

trait MyServiceSupport extends TestContainersForAll { this: Suite =>

  override type Containers = PostgreSQLContainer and ElasticsearchContainer

  override def startContainers(): Containers = {
    val container1 = PostgreSQLContainer.Def().start()
    val container2 = ElasticsearchContainer.Def().start()
    container1 and container2
  }

  private var myService: MyService = _

  override def afterContainersStart(containers: Containers): Unit = {
    super.afterContainersStart(containers)
    val pg and es = containers
    myService = new MyService(pg, es)
  }

  def withService[A](runTest: (MyService, Containers) => A): A = withContainers { containers: Containers =>
    runTest(myService, containers)
  }
}

class MyServiceTests2 extends FreeSpec with MyServiceSupport {
  "it" - {
    "should work" in withService { case (service, pg and es) =>
      // test code here
    }
  }
}

There is also beforeContainersStop method to shutdown MyService if needed.

All this design is based on the existing scalatest functionality and practices. afterContainersStart and beforeContainersStop is pretty similar to beforeAll and afterAll methods from the scalatest.

But overall I understand your frustration. Scalatest is pretty poor in terms of resource management, code reuse, and composition. It heavily relies on the oop-style inheritance, making it hard to create a composable and safe API for the resources like containers. One of the notorious problems scalatest has that testcontainers suffer from — there is no way to reuse containers between test classes. But scalatest is still the most popular test framework in the scala community. So, we have what we have.

One last mention: you are not forced to use helper traits like TestContainersForAll. You can create your own way to manage the container's lifecycle. Helper traits like TestContainersForAll help with the most common situations, but they are not necessary for testcontainers.

LMnet avatar May 14 '21 05:05 LMnet

@LMnet yes, this can be solved with using var but that is a general code smell in Scala that I would prefer to avoid. With the old API syntax, you are free to start the containers when it's appropriate and use them when it's appropriate without having to use a var or a a fixture in every test block. For our use case anyways, it's just a lot of added complexity that we don't need.

Anyways, I know I am free to make my own scalatest traits, just wanted to provide some feedback that the new API is more difficult and confusing for us to use. The inclination for new devs is to use the new API syntax, but then they get confused about how to use it, especially when they reference tests that are using the old syntax.

EDIT: this is the workaround I will use for now (for a Lagom application):

trait ApplicationSpec[Application <: LagomApplication] extends ForAllContainerSpec {
  this: Suite =>

  lazy val server: TestServer[Application] = withContainers(startServer)

  def startServer(containers: Containers): TestServer[Application]

  override def beforeContainersStop(containers: Containers): Unit = {
    super.beforeContainersStop(containers)
    server.stop()
  }
}

The server is only referenced inside the test blocks, so it should work fine.

solarmosaic-kflorence avatar May 14 '21 16:05 solarmosaic-kflorence

var but that is a general code smell in Scala that I would prefer to avoid.

The main problem of the old API is that containers are mutable (you can't distinguish between started and not started containers), which is a code smell too. New API provides another set of trade-offs here. With an old API, you have mutable containers and unsafe creation, but overall API is easier. New API is a bit more complex and cumbersome sometimes but provides more safety in terms of container usage.

LMnet avatar May 17 '21 03:05 LMnet

@LMnet definitely understand the problem the new API was addressing, and you are right that it does solve that problem with some extra complexity. I guess it's a trade off between a simpler API with the potential for implementors to use is incorrectly, vs a more complicated API that is harder to use incorrectly. I do think the use-cases for TestContainersForAll specifically could be simplified because I think it will be very common to want to use containers in conjunction with other code (e.g. applications) prior to tests being executed.

solarmosaic-kflorence avatar May 17 '21 17:05 solarmosaic-kflorence

is this issue resolved?

8bitreid avatar Jun 17 '23 04:06 8bitreid