[JVM][Spring] Constructor injection doesn't seem to work

[Seb Rose]

I've been trying to understand the Cucumber/Spring integration (helped massively by Paolo) and have hit an issue. Everything's working fine for field injection, but soon as I switch to constructor injection I get the following error:

cucumber.runtime.CucumberException: wrong number of arguments

at cucumber.runtime.java.spring.SpringFactory.getTestInstance(SpringFactory.java:162)

at cucumber.runtime.java.spring.SpringFactory.getInstance(SpringFactory.java:142)

at cucumber.runtime.java.JavaStepDefinition.execute(JavaStepDefinition.java:35)

I've put an example on github at https://github.com/sebrose/spring-dummy.git

How should it work? Should it work? Will it work?

Thanks

Seb

[Roberto Lo Giacco]

having a look right now, but from the exception I would say the cucumber.runtime.java.spring.SpringFactory is not querying the Spring Context for a bean instance, instead it is instantiating one and then asking Spring to "populate" it's dependencies.

[Roberto Lo Giacco]

I confirm it is relative to Cucumber Spring implementation: we instantiate a step definition by instantiating it's no argument constructor: if that's missing, like in your example, then you get that error.

I wouldn't consider this a major issue, it might even be considered good practice to have no argument constructors. There is a solution to this, but it has to be applied withing the SpringFactory class.... Do we want to support this? 

[James Green]

I am forced to provide no-arg ctors for some Spring and JEE classes because the framework creates a proxy. Notice I said "forced", this is something required by the framework not of my design which I would prefer not to have to provide.

[Roberto Lo Giacco]

Offending lines following

    @SuppressWarnings("unchecked")

    protected <T> T createTest(Class<T> type) throws Exception {

        return (T) type.getConstructors()[0].newInstance();

    }

 On a second thought I believe this code might require a fix anyway as it doesn't pick the no-argument constructor, but the first constructor returned by the introspector: in a multi constructor class there's no guarantee the no-argument constructor will be the first one. Just replacing the above with type.getConstructor().newInstance()should, at least, work consistently.

Aslak, do you prefer to apply this yourself or do you prefer a pull request?

[Roberto Lo Giacco]

Do you refer to the Cucumber-JVM framework or some other framework you use? 

[James Green]

To Spring and JEE. Which Cucumber has been instructed to use (Spring, in my currently active project).

[Paolo Ambrosio]

Even if personally I use and like default constructors and field
injections in step definitions (but constructor injections for
production code!), I disagree that it is good coding practice. I see
it just as a matter of preference.

I would like it to instantiate a Spring bean, handling constructor
injection as well, but I cannot understand the current implementation
enough to implement it.

> Offending lines following
>
> @SuppressWarnings("unchecked")
> protected <T> T createTest(Class<T> type) throws Exception {
> return (T) type.getConstructors()[0].newInstance();
> }
> On a second thought I believe this code might require a fix anyway as it
> doesn't pick the no-argument constructor, but the first constructor returned
> by the introspector: in a multi constructor class there's no guarantee the
> no-argument constructor will be the first one.

I was hoping to be reading it wrong.

> Just replacing the above with
> type.getConstructor().newInstance()should, at least, work consistently.

Yes, but a NullPointerException is not a very good way of
communicating the error to the user if there is no such constructor
;-)

> Aslak, do you prefer to apply this yourself or do you prefer a pull request?

My preference is for issue in the tracker + unit test + implementation
+ pull request to be honest.


Paolo

[Paolo Ambrosio]

For this simple case i guess the issue is not necessary, as it can be
exmplained easily in the pull request.

> Paolo

[Roberto Lo Giacco]

Ok Paolo,

I'm on it ;)

> Paolo

--
Posting rules: http://cukes.info/posting-rules.html
---
You received this message because you are subscribed to a topic in the Google Groups "Cukes" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/cukes/q95I0s1r80g/unsubscribe.
To unsubscribe from this group and all its topics, send an email to cukes+un...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Seb Rose]

 

While I agree that this is not a major issue, it does make the Cucumber-Spring integration different from all the other integrations.

 

PicoContainer instantiates non-default steps out of the box.

 

Guice and Weld require an @Inject annotation on the relevant constructor, but it's exactly the same annotation as you would use for field injection.

 

EITHER:

 

public class MySteps {

 

  @Inject

  private MyData myField;

 

  // snip

}

 

OR:

 

public class MySteps {

 

  private MyData myField;

 

  @Inject

  public MySteps(MyData data) { myField = data; }

 

  // snip

}

 

 

 

 

Shouldn't we aim for the same behaviour in the Spring integration?

 

 

Cheers

Seb

 

--

Posting rules: http://cukes.info/posting-rules.html

---

You received this message because you are subscribed to the Google Groups "Cukes" group.

To unsubscribe from this group and stop receiving emails from it, send an email to cukes+un...@googlegroups.com.

[Roberto Lo Giacco]

I'm already working on this, aiming at using spring framework bean initialization procedure: something I never didi before but I'm pretty sure is somewhere in there :)

You received this message because you are subscribed to a topic in the Google Groups "Cukes" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/cukes/q95I0s1r80g/unsubscribe.
To unsubscribe from this group and all its topics, send an email to cukes+un...@googlegroups.com.

[Seb Rose]

 

 

 

On Wed, 30 Apr 2014, at 04:52 AM, Roberto Lo Giacco wrote:

I'm already working on this, aiming at using spring framework bean initialization procedure: something I never didi before but I'm pretty sure is somewhere in there :)

 

Cool!

[Mykola Gurov]

 

EITHER:

 

public class MySteps {

 

  @Inject

  private MyData myField;

 

  // snip

}

 

OR:

 

public class MySteps {

 

  private MyData myField;

 

  @Inject

  public MySteps(MyData data) { myField = data; }

 

  // snip

}

 

 

 

 

Shouldn't we aim for the same behaviour in the Spring integration?

 

Here we should IMHO distinguish between the two types of dependencies. First type is beans defined in the tested spring context (via @ContextConfiguration or @ContextHierarchy sprint test annotation). I guess the initial message in this topic was about such case. Constructor injection does not seem to be supported by Spring's SpringJUnit4ClassRunner either, so I would not consider lack of this functionality as a major loss. 

What is much more interesting for the cucumber tests is the second type of the dependency - injection of the StepDef files. It is not supported by the Spring implementation at the moment. Looks like it was or intended to be supported via the custom cucumber.runtime.java.spring.SpringFactory#applicationContext , but at least since the transition to the Spring's TestContextManager, this custom context doesn't seem to have any effect (together with GlueCodeScope and GlueCodeContext). 

My cucumber tests at work rely heavily on the feature of StepDefs injection and I had to opt for the PicoContainer-driven cucumber for the time being. I tried to hack around the cucumber-jvm SpringFactory, and the best result was so far to load test contexts mentioned at @ContextConfiguration into the cucumber's SpringFactory#applicationContext and register StepDefs there, but this smells hacky and besides the functionality of DirtiesContextTestExecutionListener and ServletTestExecutionListener from the org.springframework.test.context.TestContextManager#DEFAULT_TEST_EXECUTION_LISTENER_CLASS_NAMES should be replicated. 

I believe the problem with the Spring transactional hooks might also be related to this discussion, see https://github.com/cucumber/cucumber-jvm/pull/649 .

 

[Björn Rasmusson]

Mykola Gurov wrote:

 

EITHER:

 

public class MySteps {

 

  @Inject

  private MyData myField;

 

  // snip

}

 

OR:

 

public class MySteps {

 

  private MyData myField;

 

  @Inject

  public MySteps(MyData data) { myField = data; }

 

  // snip

}

 

 

 

 

Shouldn't we aim for the same behaviour in the Spring integration?

 

Here we should IMHO distinguish between the two types of dependencies. First type is beans defined in the tested spring context (via @ContextConfiguration or @ContextHierarchy sprint test annotation). I guess the initial message in this topic was about such case. Constructor injection does not seem to be supported by Spring's SpringJUnit4ClassRunner either, so I would not consider lack of this functionality as a major loss. 

What is much more interesting for the cucumber tests is the second type of the dependency - injection of the StepDef files. It is not supported by the Spring implementation at the moment. Looks like it was or intended to be supported via the custom cucumber.runtime.java.spring.SpringFactory#applicationContext , but at least since the transition to the Spring's TestContextManager, this custom context doesn't seem to have any effect (together with GlueCodeScope and GlueCodeContext). 

AFAIK, to get StepDefs injection to work (in v1.1.4-v1.1.6), the step definition classes to be injected need either:
- to have the @Component annotation, or
- to declared as beans in the xml file specified in the @ContextConfiguration annotation (usually "classpath:cucumber.xml").
 

My cucumber tests at work rely heavily on the feature of StepDefs injection and I had to opt for the PicoContainer-driven cucumber for the time being. I tried to hack around the cucumber-jvm SpringFactory, and the best result was so far to load test contexts mentioned at @ContextConfiguration into the cucumber's SpringFactory#applicationContext and register StepDefs there, but this smells hacky and besides the functionality of DirtiesContextTestExecutionListener and ServletTestExecutionListener from the org.springframework.test.context.TestContextManager#DEFAULT_TEST_EXECUTION_LISTENER_CLASS_NAMES should be replicated. 

I believe the problem with the Spring transactional hooks might also be related to this discussion, see https://github.com/cucumber/cucumber-jvm/pull/649 .

The unintentional split into the two separate spring context, SpringFactory#applicationContext and the context the TestContextManagers create from the @ContextConfiguration, have indeed caused a set of problems. To also use a TextContextManager to apply the same context as for the step definitions, to the SpringTransactionHooks instance, is an ugly hack - but it makes the Spring transactional hooks to work, see #644.

Best Regards
Björn

 

[Mykola Gurov]

Op donderdag 1 mei 2014 18:52:15 UTC+2 schreef Björn Rasmusson:

AFAIK, to get StepDefs injection to work (in v1.1.4-v1.1.6), the step definition classes to be injected need either:
- to have the @Component annotation, or
- to declared as beans in the xml file specified in the @ContextConfiguration annotation (usually "classpath:cucumber.xml").

AFAIR, the injected StepDefs (instantiated by the TestContextManagers) will not be the ones that cucumber runner will use (instantiated by the SpringFactory itself), and thus not of much use I'm afraid.