Deprecate old lifecycle methods (#63)

Author: LMnet

README.md

@@ -256,6 +256,11 @@ class MysqlSpec extends FlatSpec with ForAllTestContainer {
 
 ## Release notes
 
+* **0.27.0**
+    * New `TestLifecycleAware` trait introduced. You can use it when you want to do something with the container before or after the test.
+    * `Container` now implements `Startable` interface with `start` and `stop` methods.
+    * Old container's lifecycle methods `finished`, `succeeded`, `starting`, `failed` are deprecated. Use `start`, `stop`, and `TestLifecycleAware` methods instead.
+
 * **0.26.0**
     * TestContainers `1.11.2` -> `1.11.3`
     * Scala 2.13.0

src/main/scala/com/dimafeng/testcontainers/DockerComposeContainer.scala

@@ -111,4 +111,7 @@ class DockerComposeContainer (composeFiles: ComposeFile,
 
   def getServicePort(serviceName: String, servicePort: Int): Int = container.getServicePort(serviceName, servicePort)
 
-}
+  override def start(): Unit = container.start()
+
+  override def stop(): Unit = container.stop()
+}

src/main/scala/com/dimafeng/testcontainers/MultipleContainers.scala

@@ -1,18 +1,36 @@
 package com.dimafeng.testcontainers
 
+import com.dimafeng.testcontainers.lifecycle.TestLifecycleAware
 import org.junit.runner.Description
+import org.testcontainers.lifecycle.TestDescription
 
 import scala.language.implicitConversions
 
-class MultipleContainers private(containers: Seq[LazyContainer[_]]) extends Container {
+class MultipleContainers private(containers: Seq[LazyContainer[_]]) extends Container with TestLifecycleAware {
 
+  @deprecated("Use `stop` instead")
   override def finished()(implicit description: Description): Unit = containers.foreach(_.finished()(description))
 
+  @deprecated("Use `stop` and/or `TestLifecycleAware.afterTest` instead")
   override def succeeded()(implicit description: Description): Unit = containers.foreach(_.succeeded()(description))
 
+  @deprecated("Use `start` instead")
   override def starting()(implicit description: Description): Unit = containers.foreach(_.starting()(description))
 
+  @deprecated("Use `stop` and/or `TestLifecycleAware.afterTest` instead")
   override def failed(e: Throwable)(implicit description: Description): Unit = containers.foreach(_.failed(e)(description))
+
+  override def beforeTest(description: TestDescription): Unit = {
+    containers.foreach(_.beforeTest(description))
+  }
+
+  override def afterTest(description: TestDescription, throwable: Option[Throwable]): Unit = {
+    containers.foreach(_.afterTest(description, throwable))
+  }
+
+  override def start(): Unit = containers.foreach(_.start())
+
+  override def stop(): Unit = containers.foreach(_.stop())
 }
 
 object MultipleContainers {
@@ -47,16 +65,38 @@ object MultipleContainers {
   * You don't need to wrap your containers into the `LazyContainer` manually
   * when you pass your containers in the `MultipleContainers`- there is implicit conversion for that.
   */
-class LazyContainer[T <: Container](factory: => T) extends Container {
+class LazyContainer[T <: Container](factory: => T) extends Container with TestLifecycleAware {
   lazy val container: T = factory
 
+  @deprecated("Use `stop` instead")
   override def finished()(implicit description: Description): Unit = container.finished
 
+  @deprecated("Use `stop` and/or `TestLifecycleAware.afterTest` instead")
   override def failed(e: Throwable)(implicit description: Description): Unit = container.failed(e)
 
+  @deprecated("Use `start` instead")
   override def starting()(implicit description: Description): Unit = container.starting()
 
+  @deprecated("Use `stop` and/or `TestLifecycleAware.afterTest` instead")
   override def succeeded()(implicit description: Description): Unit = container.succeeded()
+
+  override def beforeTest(description: TestDescription): Unit = {
+    container match {
+      case c: TestLifecycleAware => c.beforeTest(description)
+      case _ => // do nothing
+    }
+  }
+
+  override def afterTest(description: TestDescription, throwable: Option[Throwable]): Unit = {
+    container match {
+      case c: TestLifecycleAware => c.afterTest(description, throwable)
+      case _ => // do nothing
+    }
+  }
+
+  override def start(): Unit = container.start()
+
+  override def stop(): Unit = container.stop()
 }
 
 object LazyContainer {

src/main/scala/com/dimafeng/testcontainers/SeleniumTestContainerSuite.scala

@@ -2,11 +2,14 @@ package com.dimafeng.testcontainers
 
 import java.io.File
 import java.net.URL
+import java.util.Optional
 
+import com.dimafeng.testcontainers.lifecycle.TestLifecycleAware
 import org.openqa.selenium.WebDriver
 import org.openqa.selenium.remote.{DesiredCapabilities, RemoteWebDriver}
 import org.scalatest.Suite
 import org.testcontainers.containers.BrowserWebDriverContainer
+import org.testcontainers.lifecycle.TestDescription
 
 
 trait SeleniumTestContainerSuite extends ForEachTestContainer {
@@ -23,7 +26,7 @@ trait SeleniumTestContainerSuite extends ForEachTestContainer {
 
 class SeleniumContainer(desiredCapabilities: Option[DesiredCapabilities] = None,
                         recordingMode: Option[(BrowserWebDriverContainer.VncRecordingMode, File)] = None)
-  extends SingleContainer[BrowserWebDriverContainer[_]] {
+  extends SingleContainer[BrowserWebDriverContainer[_]] with TestLifecycleAware {
   require(desiredCapabilities.isDefined, "'desiredCapabilities' is required parameter")
 
   type OTCContainer = BrowserWebDriverContainer[T] forSome {type T <: BrowserWebDriverContainer[T]}
@@ -40,6 +43,14 @@ class SeleniumContainer(desiredCapabilities: Option[DesiredCapabilities] = None,
   def vncAddress: String = container.getVncAddress
 
   def webDriver: RemoteWebDriver = container.getWebDriver
+
+  override def afterTest(description: TestDescription, throwable: Option[Throwable]): Unit = {
+    val javaThrowable: Optional[Throwable] = throwable match {
+      case Some(error) => Optional.of(error)
+      case None => Optional.empty()
+    }
+    container.afterTest(description, javaThrowable)
+  }
 }
 
 object SeleniumContainer {

src/main/scala/com/dimafeng/testcontainers/TestContainer.scala

@@ -2,6 +2,9 @@ package com.dimafeng.testcontainers
 
 import java.util.function.Consumer
 
+import com.dimafeng.testcontainers.TestContainers.TestContainersSuite
+import com.dimafeng.testcontainers.lifecycle.TestLifecycleAware
+import org.junit.runner.{Description => JunitDescription}
 import com.github.dockerjava.api.DockerClient
 import com.github.dockerjava.api.command.{CreateContainerCmd, InspectContainerResponse}
 import com.github.dockerjava.api.model.{Bind, Info, VolumesFrom}
@@ -11,60 +14,113 @@ import org.testcontainers.containers.output.OutputFrame
 import org.testcontainers.containers.startupcheck.StartupCheckStrategy
 import org.testcontainers.containers.traits.LinkableContainer
 import org.testcontainers.containers.{FailureDetectingExternalResource, Network, TestContainerAccessor, GenericContainer => OTCGenericContainer}
+import org.testcontainers.lifecycle.{Startable, TestDescription}
 
 import scala.collection.JavaConverters._
 import scala.concurrent.{Future, blocking}
 
-trait ForEachTestContainer extends SuiteMixin {
-  self: Suite =>
+private[testcontainers] object TestContainers {
+
+  implicit def junit2testContainersDescription(junit: JunitDescription): TestDescription = {
+    new TestDescription {
+      override def getTestId: String = junit.getDisplayName
+      override def getFilesystemFriendlyName: String = s"${junit.getClassName}-${junit.getMethodName}"
+    }
+  }
+
+  // Copy-pasted from `org.scalatest.junit.JUnitRunner.createDescription`
+  def createDescription(suite: Suite): JunitDescription = {
+    val description = JunitDescription.createSuiteDescription(suite.getClass)
+    // If we don't add the testNames and nested suites in, we get
+    // Unrooted Tests show up in Eclipse
+    for (name <- suite.testNames) {
+      description.addChild(JunitDescription.createTestDescription(suite.getClass, name))
+    }
+    for (nestedSuite <- suite.nestedSuites) {
+      description.addChild(createDescription(nestedSuite))
+    }
+    description
+  }
+
+  trait TestContainersSuite extends SuiteMixin { self: Suite =>
+
+    val container: Container
+
+    def afterStart(): Unit = {}
+
+    def beforeStop(): Unit = {}
+
+    private val suiteDescription = createDescription(self)
 
-  val container: Container
+    private[testcontainers] def beforeTest(): Unit = {
+      container match {
+        case container: TestLifecycleAware => container.beforeTest(suiteDescription)
+        case _ => // do nothing
+      }
+    }
 
-  implicit private val suiteDescription = Description.createSuiteDescription(self.getClass)
+    private[testcontainers] def afterTest(throwable: Option[Throwable]): Unit = {
+      container match {
+        case container: TestLifecycleAware => container.afterTest(suiteDescription, throwable)
+        case _ => // do nothing
+      }
+    }
+  }
+}
+
+trait ForEachTestContainer extends TestContainersSuite {
+  self: Suite =>
 
   abstract protected override def runTest(testName: String, args: Args): Status = {
-    container.starting()
+    container.start()
+
+    @volatile var testCalled = false
+    @volatile var afterTestCalled = false
+
     try {
       afterStart()
+      beforeTest()
+
+      testCalled = true
       val status = super.runTest(testName, args)
-      status match {
-        case FailedStatus => container.failed(new RuntimeException(status.toString))
-        case _ => container.succeeded()
+
+      afterTestCalled = true
+      if (!status.succeeds()) {
+        afterTest(Some(new RuntimeException("Test failed")))
+      } else {
+        afterTest(None)
       }
+
       status
     }
     catch {
       case e: Throwable =>
-        container.failed(e)
+        if (testCalled && !afterTestCalled) {
+          afterTestCalled = true
+          afterTest(Some(e))
+        }
+
         throw e
     }
     finally {
       try {
         beforeStop()
       }
       finally {
-        container.finished()
+        container.stop()
       }
     }
   }
-
-  def afterStart(): Unit = {}
-
-  def beforeStop(): Unit = {}
 }
 
-trait ForAllTestContainer extends SuiteMixin {
+trait ForAllTestContainer extends TestContainersSuite {
   self: Suite =>
 
-  val container: Container
-
-  implicit private val suiteDescription = Description.createSuiteDescription(self.getClass)
-
   abstract override def run(testName: Option[String], args: Args): Status = {
     if (expectedTestCount(args.filter) == 0) {
       new CompositeStatus(Set.empty)
     } else {
-      container.starting()
+      container.start()
       try {
         afterStart()
         super.run(testName, args)
@@ -73,43 +129,82 @@ trait ForAllTestContainer extends SuiteMixin {
           beforeStop()
         }
         finally {
-          container.finished()
+          container.stop()
         }
       }
     }
   }
 
-  def afterStart(): Unit = {}
+  abstract protected override def runTest(testName: String, args: Args): Status = {
+    @volatile var testCalled = false
+    @volatile var afterTestCalled = false
+
+    try {
+      beforeTest()
+
+      testCalled = true
+      val status = super.runTest(testName, args)
+
+      afterTestCalled = true
+      if (!status.succeeds()) {
+        afterTest(Some(new RuntimeException("Test failed")))
+      } else {
+        afterTest(None)
+      }
 
-  def beforeStop(): Unit = {}
+      status
+    }
+    catch {
+      case e: Throwable =>
+        if (testCalled && !afterTestCalled) {
+          afterTestCalled = true
+          afterTest(Some(e))
+        }
+
+        throw e
+    }
+  }
 }
 
-trait Container {
-  def finished()(implicit description: Description): Unit
+trait Container extends Startable {
 
-  def failed(e: Throwable)(implicit description: Description): Unit
+  @deprecated("Use `stop` instead")
+  def finished()(implicit description: Description): Unit = stop()
 
-  def starting()(implicit description: Description): Unit
+  @deprecated("Use `stop` and/or `TestLifecycleAware.afterTest` instead")
+  def failed(e: Throwable)(implicit description: Description): Unit = {}
 
-  def succeeded()(implicit description: Description): Unit
+  @deprecated("Use `start` instead")
+  def starting()(implicit description: Description): Unit = start()
+
+  @deprecated("Use `stop` and/or `TestLifecycleAware.afterTest` instead")
+  def succeeded()(implicit description: Description): Unit = {}
 }
 
 trait TestContainerProxy[T <: FailureDetectingExternalResource] extends Container {
 
   @deprecated("Please use reflective methods from the wrapper and `configure` method for creation")
-  implicit val container: T
+  implicit def container: T
 
+  @deprecated("Use `stop` instead")
   override def finished()(implicit description: Description): Unit = TestContainerAccessor.finished(description)
 
+  @deprecated("Use `stop` and/or `TestLifecycleAware.afterTest` instead")
   override def succeeded()(implicit description: Description): Unit = TestContainerAccessor.succeeded(description)
 
+  @deprecated("Use `start` instead")
   override def starting()(implicit description: Description): Unit = TestContainerAccessor.starting(description)
 
+  @deprecated("Use `stop` and/or `TestLifecycleAware.afterTest` instead")
   override def failed(e: Throwable)(implicit description: Description): Unit = TestContainerAccessor.failed(e, description)
 }
 
 abstract class SingleContainer[T <: OTCGenericContainer[_]] extends TestContainerProxy[T] {
 
+  override def start(): Unit = container.start()
+
+  override def stop(): Unit = container.stop()
+
   def binds: Seq[Bind] = container.getBinds.asScala.toSeq
 
   def command: Seq[String] = container.getCommandParts

src/main/scala/com/dimafeng/testcontainers/lifecycle/TestLifecycleAware.scala

@@ -0,0 +1,10 @@
+package com.dimafeng.testcontainers.lifecycle
+
+import org.testcontainers.lifecycle.TestDescription
+
+trait TestLifecycleAware {
+
+  def beforeTest(description: TestDescription): Unit = {}
+
+  def afterTest(description: TestDescription, throwable: Option[Throwable]): Unit = {}
+}

src/main/scala/org/testcontainers/containers/TestContainerAccessor.scala

@@ -2,6 +2,7 @@ package org.testcontainers.containers
 
 import org.junit.runner.Description
 
+@deprecated("Should be replaced by lifecycle methods")
 object TestContainerAccessor {
   def finished[T <:FailureDetectingExternalResource](description: Description)(implicit container: T): Unit =
     container.finished(description)