Allow usage of class with args and dependency injection (#292)

* test: add integration tests for picocontainer

* fix: better identification of class vs object

As a side effect, this allows to work with classes having args constructor

* docs: update CHANGELOG

* docs: document DI ability

Author: gaeljw

CHANGELOG.md

@@ -19,6 +19,8 @@ See also the [CHANGELOG](https://github.com/cucumber/cucumber-jvm/blob/master/CH
 
 ### Fixed
 
+- [Scala] Integration with DI modules like `cucumber-picocontainer` is now working
+
 ## [8.3.3] (2022-05-04)
 
 ### Changed

build.sbt

@@ -66,6 +66,7 @@ lazy val root = (project in file("."))
     cucumberScala.projectRefs ++
       integrationTestsCommon.projectRefs ++
       integrationTestsJackson.projectRefs ++
+      integrationTestsPicoContainer.projectRefs ++
       examples.projectRefs: _*
   )
 
@@ -151,6 +152,21 @@ lazy val integrationTestsJackson =
     .dependsOn(cucumberScala % Test)
     .jvmPlatform(scalaVersions = Seq(scala3, scala213, scala212))
 
+lazy val integrationTestsPicoContainer =
+  (projectMatrix in file("integration-tests/picocontainer"))
+    .settings(commonSettings)
+    .settings(
+      name := "integration-tests-picocontainer",
+      libraryDependencies ++= Seq(
+        "junit" % "junit" % junitVersion % Test,
+        "io.cucumber" % "cucumber-junit" % cucumberVersion % Test,
+        "io.cucumber" % "cucumber-picocontainer" % cucumberVersion % Test
+      ),
+      publishArtifact := false
+    )
+    .dependsOn(cucumberScala % Test)
+    .jvmPlatform(scalaVersions = Seq(scala3, scala213, scala212))
+
 // Examples project
 lazy val examples = (projectMatrix in file("examples"))
   .settings(commonSettings)

cucumber-scala/src/main/scala/io/cucumber/scala/ScalaBackend.scala

@@ -1,15 +1,32 @@
 package io.cucumber.scala
 
+import io.cucumber.core.backend._
+import io.cucumber.core.resource.{ClasspathScanner, ClasspathSupport}
+import io.cucumber.scala.ScalaBackend.isRegularClass
+
 import java.lang.reflect.Modifier
 import java.net.URI
 import java.util.function.Supplier
 import java.util.{List => JList}
-
-import io.cucumber.core.backend._
-import io.cucumber.core.resource.{ClasspathScanner, ClasspathSupport}
-
 import scala.jdk.CollectionConverters._
-import scala.util.Try
+import scala.util.{Failure, Try}
+
+object ScalaBackend {
+
+  /** @return true if it's a class, false if it's an object
+    */
+  private[scala] def isRegularClass(cls: Class[_]): Try[Boolean] = {
+    Try {
+      // Object don't have constructors
+      cls.getConstructors.headOption
+        .map(_.getModifiers)
+        .exists(Modifier.isPublic)
+    }.recoverWith { case ex: Throwable =>
+      Failure(new UnknownClassType(cls, ex))
+    }
+  }
+
+}
 
 class ScalaBackend(
     lookup: Lookup,
@@ -33,8 +50,15 @@ class ScalaBackend(
   override def buildWorld(): Unit = {
     // Instantiate all the glue classes and load the glue code from them
     scalaGlueClasses.foreach { glueClass =>
-      val glueInstance = lookup.getInstance(glueClass)
-      glueAdaptor.loadRegistry(glueInstance.registry, scenarioScoped = true)
+      val glueInstance = Option(lookup.getInstance(glueClass))
+      glueInstance match {
+        case Some(glue) =>
+          glueAdaptor.loadRegistry(glue.registry, scenarioScoped = true)
+        case None =>
+          throw new CucumberBackendException(
+            s"Not able to instantiate class ${glueClass.getName}. Please report this issue to cucumber-scala project."
+          )
+      }
     }
   }
 
@@ -54,7 +78,9 @@ class ScalaBackend(
       )
       .filter(glueClass => !glueClass.isInterface)
 
-    val (clsClasses, objClasses) = dslClasses.partition(isRegularClass)
+    // Voluntarily throw exception if not able to identify if it's a class
+    val (clsClasses, objClasses) =
+      dslClasses.partition(c => isRegularClass(c).get)
 
     // Retrieve Scala objects (singletons)
     val objInstances = objClasses.map { cls =>
@@ -78,8 +104,4 @@ class ScalaBackend(
     ()
   }
 
-  private def isRegularClass(cls: Class[_]): Boolean = {
-    Try(Modifier.isPublic(cls.getConstructor().getModifiers)).getOrElse(false)
-  }
-
 }

cucumber-scala/src/main/scala/io/cucumber/scala/UnknownClassType.scala

@@ -0,0 +1,9 @@
+package io.cucumber.scala
+
+import io.cucumber.core.backend.CucumberBackendException
+
+class UnknownClassType(clazz: Class[_], cause: Throwable)
+    extends CucumberBackendException(
+      s"Cucumber was not able to handle class ${clazz.getName}. Please report this issue to cucumber-scala project.",
+      cause
+    )

cucumber-scala/src/test/scala/io/cucumber/scala/ScalaBackendTest.scala

@@ -2,8 +2,10 @@ package io.cucumber.scala
 
 import io.cucumber.core.backend._
 import io.cucumber.scala.steps.classes.{StepsA, StepsB, StepsC}
+import io.cucumber.scala.steps.dependencyinjection.{Injected, Injector}
 import io.cucumber.scala.steps.errors.incorrectclasshooks.IncorrectClassHooksDefinition
 import io.cucumber.scala.steps.errors.staticclasshooks.StaticClassHooksDefinition
+import io.cucumber.scala.steps.objects.StepsInObject
 import io.cucumber.scala.steps.traits.StepsInTrait
 import org.junit.Assert.{assertEquals, assertTrue, fail}
 import org.junit.{Before, Test}
@@ -43,6 +45,9 @@ class ScalaBackendTest {
       .thenReturn(new IncorrectClassHooksDefinition())
     when(fakeLookup.getInstance(classOf[StaticClassHooksDefinition]))
       .thenReturn(new StaticClassHooksDefinition())
+    when(fakeLookup.getInstance(classOf[Injected])).thenReturn(new Injected)
+    when(fakeLookup.getInstance(classOf[Injector]))
+      .thenReturn(new Injector(new Injected))
 
     // Create the instances
     backend = new ScalaBackend(fakeLookup, fakeContainer, classLoaderSupplier)
@@ -317,4 +322,66 @@ class ScalaBackendTest {
     }
   }
 
+  @Test
+  def loadGlueAndBuildWorld_DI(): Unit = {
+    // Load glue
+    backend.loadGlue(
+      fakeGlue,
+      List(
+        URI.create("classpath:io/cucumber/scala/steps/dependencyinjection")
+      ).asJava
+    )
+
+    assertEquals(2, backend.scalaGlueClasses.size)
+    assertTrue(
+      backend.scalaGlueClasses.toSet == Set(
+        classOf[Injected],
+        classOf[Injector]
+      )
+    )
+
+    verify(fakeContainer, times(2)).addClass(any())
+    verify(fakeContainer, times(1)).addClass(classOf[Injected])
+    verify(fakeContainer, times(1)).addClass(classOf[Injector])
+
+    // Build world
+    backend.buildWorld()
+
+    verify(fakeLookup, times(2)).getInstance(any())
+    verify(fakeLookup, times(1)).getInstance(classOf[Injected])
+    verify(fakeLookup, times(1)).getInstance(classOf[Injector])
+
+    // Building the world a second time should create new instances
+    backend.disposeWorld()
+    backend.buildWorld()
+
+    verify(fakeLookup, times(4)).getInstance(any())
+    verify(fakeLookup, times(2)).getInstance(classOf[Injected])
+    verify(fakeLookup, times(2)).getInstance(classOf[Injector])
+
+  }
+
+  @Test
+  def isRegularClass_class(): Unit = {
+    assertEquals(true, ScalaBackend.isRegularClass(classOf[StepsA]).get)
+  }
+
+  @Test
+  def isRegularClass_class_with_trait(): Unit = {
+    assertEquals(true, ScalaBackend.isRegularClass(classOf[StepsInTrait]).get)
+  }
+
+  @Test
+  def isRegularClass_class_with_args(): Unit = {
+    assertEquals(true, ScalaBackend.isRegularClass(classOf[Injector]).get)
+  }
+
+  @Test
+  def isRegularClass_object(): Unit = {
+    assertEquals(
+      false,
+      ScalaBackend.isRegularClass(StepsInObject.getClass).get
+    )
+  }
+
 }

cucumber-scala/src/test/scala/io/cucumber/scala/steps/dependencyinjection/Injected.scala

@@ -0,0 +1,13 @@
+package io.cucumber.scala.steps.dependencyinjection
+
+import io.cucumber.scala.{EN, ScalaDsl}
+
+class Injected extends ScalaDsl with EN {
+
+  var x: String = _
+
+  Given("""injected steps""") { () =>
+    // Nothing
+  }
+
+}

cucumber-scala/src/test/scala/io/cucumber/scala/steps/dependencyinjection/Injector.scala

@@ -0,0 +1,11 @@
+package io.cucumber.scala.steps.dependencyinjection
+
+import io.cucumber.scala.{EN, ScalaDsl}
+
+class Injector(injected: Injected) extends ScalaDsl with EN {
+
+  Then("""Injector""") { () =>
+    println(injected.x)
+  }
+
+}

docs/usage.md

@@ -50,6 +50,37 @@ You can also define glue code in **objects**.
 Be aware that by definition objects are singleton and if your glue code is stateful you will probably have "state conflicts"
 between your scenarios if you use shared variables from objects.
 
+### Using dependency-injection
+
+Starting with cucumber-scala 8.4, it is possible to use DI modules in order to share state between steps.
+
+You can for instance have the following definition:
+```scala
+import io.cucumber.scala.{EN, ScalaDsl}
+
+class A extends ScalaDsl with EN {
+
+  var input: String = _
+
+  Given("""a step defined in class A with arg {string}""") { (arg: String) =>
+    input = arg
+  }
+
+}
+
+class B(a: A) extends ScalaDsl with EN {
+
+  When("""a step defined in class B uses A""") { () =>
+    // Do something with a.input
+    println(a.input)
+  }
+
+}
+```
+
+To make it work, you only need a Cucumber DI module to be added as a dependency of your project
+(like `cucumber-picocontainer`, or `cucumber-guice`, or any other provided by Cucumber).
+
 ## Running Cucumber tests
 
 See also the Running Cucumber for Java [documentation](https://docs.cucumber.io/docs/cucumber/api/#running-cucumber).

integration-tests/picocontainer/src/test/resources/cucumber.properties

@@ -0,0 +1 @@
+cucumber.publish.quiet=true

integration-tests/picocontainer/src/test/resources/di/di.feature

@@ -0,0 +1,7 @@
+Feature: As Cucumber Scala, I want to be able to have some step classes depend on another one
+
+  Scenario: Nominal case
+    Given a step defined in class DI-A with arg "A"
+    And a step defined in class DI-B with arg "B"
+    When a step defined in class DI-C uses them both
+    Then both values are combined into "AB"