Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scala steps should use dependency injection, like Java #4

Closed
mpkorstanje opened this issue Oct 14, 2017 · 3 comments · Fixed by #292
Closed

Scala steps should use dependency injection, like Java #4

mpkorstanje opened this issue Oct 14, 2017 · 3 comments · Fixed by #292
Assignees
Labels
⚡ enhancement Request for new functionality
Milestone

Comments

@mpkorstanje
Copy link
Contributor

From cucumber/cucumber-jvm#1075

Summary

I have found that the Scala step classes are required to have a no-args constructor, but I'd like to be able to have my glue classes depend on one another like the Java glue can.

From the Java docs:

we strongly recommend you include one of the Dependency Injection modules as well. This allows you to share state between Step Definitions without resorting to static variables (a common source of flickering scenarios).

The relevant code is here:

val dslClasses = packages flatMap { classFinder.getDescendants(classOf[ScalaDsl], _) } filter { cls =>
  try {
    cls.getDeclaredConstructor()
    true
  } catch {
    case e : Throwable => false
  }
}

This filter will silently ignore any Step classes which do not have a no-args constructor.

See also #755 for another difference between Scala and Java here.

Expected Behavior

  1. I'd expect to be able to add a Glue class as a constructor param to a Step class, and have the DI framework create and inject an instance, like the Java API allows:
class MySteps(connector: MyConnector) {
    Given("""^some setup$"""){ () =>
      connector.something()
    }
}
  1. Also, I think that the filter above ought to print a warning when a Glue class that inherits from ScalaDsl is rejected for having the wrong constructor, as it took me a while to figure out what was going on here.

Current Behavior

  1. Any ScalaDsl classes with constructors like that shown above are not loaded into the backend
  2. No warning is currently given.

Possible Solution

I should think that most of the JavaBackend.loadGlue code could be used directly by the ScalaBackend, instead of its own less fully-featured version (some allowances are currently made in the ScalaBackend for "MODULE$" fields, but I don't immediately see why that's necessary).

Steps to Reproduce (for bugs)

Hopefully the above is clear enough; please let me know if not.

Context & Motivation

I'm starting a new Scala proejct and I'd like to use cucumber-jvm. I have experience using cucumber-jvm on Java projects and Scala experience.

Your Environment

n/a

@jschmuec
Copy link

jschmuec commented Apr 3, 2020

Just reading this and I think there is no need for this feature in Scala.

Scala usually does not need dependency injection in the same way Java does. You can just use Traits do create your steps and then bring it all together in a test class that extends all these Traits.

@mpkorstanje
Copy link
Contributor Author

That sounds like a reasonable work around for now but it be nice if the trait aggregating class wasn't needed.

@gaeljw
Copy link
Member

gaeljw commented Apr 14, 2020

some allowances are currently made in the ScalaBackend for "MODULE$" fields, but I don't immediately see why that's necessary

The MODULE$ field comes with Scala object which are by definition singletons.
I believe that is the reason why there is a special case in the code for this.

But... if we want a cleaner approach, we should not allow steps to be defined in object I guess.

@gaeljw gaeljw added the ⚡ enhancement Request for new functionality label May 10, 2020
@gaeljw gaeljw self-assigned this May 28, 2022
@gaeljw gaeljw added 🐛 bug Defect / Bug and removed 🐛 bug Defect / Bug labels May 28, 2022
@gaeljw gaeljw added this to the 8.4 milestone May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡ enhancement Request for new functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants