Cucumber-Spring - let's fix it together!

[aslak hellesoy]

Hi everyone,

The Cucumber-Spring module in cucumber-JVM is pretty broken. I broke it a while back by merging in patches without understanding what the code was doing. This module doesn't have good enough tests to detect regressions, and I personally don't have enough Spring-fu to correct what I've done, without reverting back to a rather old version of the codebase.

My limited understanding of what's broken:

* Transactions

* A context config must be specified for each glue class

If you know Spring and want to help out, please fill in your availability so we can pick a couple of time slots to discuss/hack together online.

-------->  http://whenisgood.net/ed7cw9g  <--------

The upcoming Cucumber-JVM book will have a chapter about Spring, and if you help fix any of the bugs in Cucumber-Spring we'll give you due credit in the book!

-------->  http://whenisgood.net/ed7cw9g  <--------

Cheers,

Aslak

[Miguel Almeida]

Nice one Aslak!

While my knowledge of the inner works are limited, I am definitely interested in getting this fixed as we use it in our team.
I will share this and will try to help out!

Miguel

[James Green]

On 6 May 2014 10:05, aslak hellesoy <aslak.h...@gmail.com> wrote:

Hi everyone,

The Cucumber-Spring module in cucumber-JVM is pretty broken. I broke it a while back by merging in patches without understanding what the code was doing. This module doesn't have good enough tests to detect regressions, and I personally don't have enough Spring-fu to correct what I've done, without reverting back to a rather old version of the codebase.

My limited understanding of what's broken:

* Transactions

* A context config must be specified for each glue class

I genuinely want to enquire why this is wrong.

I ask as when creating Spring integration tests Spring will, given an @ConfigurationContext, load it's context from a beans xml file who's name is based on that of the class executing the tests.

I can understand some people would not naturally expect this, but it helps ensure each class runs in it's own very limited context, exercising only the components deemed needed by the author.  It crosses my mind that educating people to this matter with Cucumber may be the more correct way forward.

Happy to overruled by someone with deeper understanding though.

If you know Spring and want to help out, please fill in your availability so we can pick a couple of time slots to discuss/hack together online.

-------->  http://whenisgood.net/ed7cw9g  <--------

The upcoming Cucumber-JVM book will have a chapter about Spring, and if you help fix any of the bugs in Cucumber-Spring we'll give you due credit in the book!

-------->  http://whenisgood.net/ed7cw9g  <--------

Cheers,

Aslak

--
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.
For more options, visit https://groups.google.com/d/optout.

[Miguel Almeida]

On Tuesday, May 6, 2014 10:27:19 AM UTC+1, James Green wrote:

On 6 May 2014 10:05, aslak hellesoy <aslak.h...@gmail.com> wrote:

Hi everyone,

The Cucumber-Spring module in cucumber-JVM is pretty broken. I broke it a while back by merging in patches without understanding what the code was doing. This module doesn't have good enough tests to detect regressions, and I personally don't have enough Spring-fu to correct what I've done, without reverting back to a rather old version of the codebase.

My limited understanding of what's broken:

* Transactions

* A context config must be specified for each glue class

I genuinely want to enquire why this is wrong.

I ask as when creating Spring integration tests Spring will, given an @ConfigurationContext, load it's context from a beans xml file who's name is based on that of the class executing the tests.

I can understand some people would not naturally expect this, but it helps ensure each class runs in it's own very limited context, exercising only the components deemed needed by the author.  It crosses my mind that ducating people to this matter with Cucumber may be the more correct way forward.

Shouldn't the context configuration be defined in the @Options though? Defining it in the the StepDefs file seems like an overkill. Similarly, when you create a spring integration test the context is defined in the _Test class and not in every helper class that the Test class uses.
Besides, what happens if you define two step def classes and each has each own configuration? From my limited knowledge, it seems something like this is creating problems in the transaction hooks (https://github.com/cucumber/cucumber-jvm/pull/649 ): if the SpringTransactionHooks class is declared as a bean in cucumber-glue.xml it will get access to a bean factory, but that bean factory will not know anything of beans and classes declared in the applications context (defined by the file in the @ContextConfiguration annotations), and therefore the SpringTransactionHooks class cannot access the PlatformTransactionManager.

[Paolo Ambrosio]

On Tue, May 6, 2014 at 10:27 AM, James Green <james.m...@gmail.com> wrote:
> On 6 May 2014 10:05, aslak hellesoy <aslak.h...@gmail.com> wrote:
>>
>> Hi everyone,
>>
>> The Cucumber-Spring module in cucumber-JVM is pretty broken. I broke it a
>> while back by merging in patches without understanding what the code was
>> doing. This module doesn't have good enough tests to detect regressions, and
>> I personally don't have enough Spring-fu to correct what I've done, without
>> reverting back to a rather old version of the codebase.
>>
>> My limited understanding of what's broken:
>>
>> * Transactions
>> * A context config must be specified for each glue class
>
>
> I genuinely want to enquire why this is wrong.

If not for any other reason, because it should at least have a default
for the 99% of the users that do not care about this flexibility.

> I ask as when creating Spring integration tests Spring will, given an
> @ConfigurationContext, load it's context from a beans xml file who's name is
> based on that of the class executing the tests.

IMO Spring integration tests are very different from Cucumber step
definitions. A step definition does not represent a scenario, but
instead it is more similar to a service layer.

> I can understand some people would not naturally expect this, but it helps
> ensure each class runs in it's own very limited context, exercising only the
> components deemed needed by the author. It crosses my mind that educating
> people to this matter with Cucumber may be the more correct way forward.

If you want to inject different collaborators in each step definition,
you can still use qualifiers. Creating a different context for each
step definition seems wrong to me.

In my opinion the context should be global to all step definitions in
a cucumber run. I see value in configuring the context to be used
instead of the predefined cucumber.xml.

[Andrew Premdas]

Perhaps this is similar in function to Cucumber::World in ruby. The idea of a global context is integral to the design of Cucumber.Ruby, but is often misunderstood and seen as bad e.g. every thing global is bad. In fact in this case the opposite is true.

All best

Andrew

--

------------------------

Andrew Premdas

blog.andrew.premdas.org

[James Green]

On 6 May 2014 11:18, Miguel Almeida <migueld...@gmail.com> wrote:

Shouldn't the context configuration be defined in the @Options though? Defining it in the the StepDefs file seems like an overkill. Similarly, when you create a spring integration test the context is defined in the _Test class and not in every helper class that the Test class uses.

Indeed if the context can be configured by the Runner class that makes sense. So what is suggested is that for each context desired, create a Runner configured with that context. Each Runner will always run the step definitions when required just in their own contexts. Thus is makes sense for the Runner to also point at a possibly limited set of feature files.

Having Runners and then a separate set of classes with step definitions spread out is rather foreign to me conceptually. I've been struggling to source a good example showing a complex application with tests designed this way to imagine our own properly.

[Björn Rasmusson]

apremdas wrote:

On 6 May 2014 13:45, Paolo Ambrosio <pa...@ambrosio.name> wrote:

On Tue, May 6, 2014 at 10:27 AM, James Green <james.m...@gmail.com> wrote:
> On 6 May 2014 10:05, aslak hellesoy <aslak.h...@gmail.com> wrote:
>>
>> Hi everyone,
>>
>> The Cucumber-Spring module in cucumber-JVM is pretty broken. I broke it a
>> while back by merging in patches without understanding what the code was
>> doing. This module doesn't have good enough tests to detect regressions, and
>> I personally don't have enough Spring-fu to correct what I've done, without
>> reverting back to a rather old version of the codebase.
>>
>> My limited understanding of what's broken:
>>
>> * Transactions

Yes, transactions are broken, an ugly hack which seems to fix it is available at #644 (detect the context config from a step def and apply it to SpringTransactionHooks).

>> * A context config must be specified for each glue class

Is it a bug or a feature? Was the intention of moving to ContextConfiguration annotations just to get rid of the hard coding of cucumber.xml, or was it to support different context on different glue classes?

* Stepdef injection does not work, see #711
I have prototyped a fix which set the SpringFactory#applicationContext as the parent context to the context defined by the ContextConfiguration annotation (this my not be the right way to go, I do not use spring myself, so my knowledge about spring is quite shallow).

* The scope cucumber-glue not available automatically to the test configuration files, see #600. The work around is import cucumber-glue.xml into the test configuration file.

Best Regards
Björn

[Björn Rasmusson]

James Green wrote:

On 6 May 2014 11:18, Miguel Almeida <migueld...@gmail.com> wrote:

Shouldn't the context configuration be defined in the @Options though? Defining it in the the StepDefs file seems like an overkill. Similarly, when you create a spring integration test the context is defined in the _Test class and not in every helper class that the Test class uses.

Indeed if the context can be configured by the Runner class that makes sense. So what is suggested is that for each context desired, create a Runner configured with that context. Each Runner will always run the step definitions when required just in their own contexts. Thus is makes sense for the Runner to also point at a possibly limited set of feature files.

A complication if using @Options or the Runner class for the information about the spring context, is to get that information from Cucumber-Junit or Cucumber-TestNG (depending on the runner used) to, Cucumber-spring. And also that spring specific information is handled by Cucumber-Junit or Cucumber-TestNG.

From that point of view, it is maybe not a better, but a more local implementation confined to Cucumber-spring, to use context information on the StepDefs.
A possible option would be to if one @ContextConfiguration is found, the that context is applied to all StepDef classes, so that they all automatically belongs to the same context.

Best Regards
Björn