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

Spring example hook path #649

Closed
wants to merge 2 commits into from
Closed

Spring example hook path #649

wants to merge 2 commits into from

Conversation

ffbit
Copy link
Contributor

@ffbit ffbit commented Dec 16, 2013

Fixed the NPE in in SpringTransactionHooks by turnign it into a Spring bean.
Got a NoSuchBeanDefinitionException instead.

org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type [org.springframework.transaction.PlatformTransactionManager] is defined
  at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBean(DefaultListableBeanFactory.java:295)
  at cucumber.api.spring.SpringTransactionHooks.obtainPlatformTransactionManager(SpringTransactionHooks.java:65)
  at cucumber.api.spring.SpringTransactionHooks.rollBackTransaction(SpringTransactionHooks.java:60)
  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  ...

Relies on the #644, #637 issues.

mmalmeida and others added 2 commits December 8, 2013 16:14
…g bean.

Got a NoSuchBeanDefinitionException instead.
````
org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type [org.springframework.transaction.PlatformTransactionManager] is defined
  at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBean(DefaultListableBeanFactory.java:295)
  at cucumber.api.spring.SpringTransactionHooks.obtainPlatformTransactionManager(SpringTransactionHooks.java:65)
  at cucumber.api.spring.SpringTransactionHooks.rollBackTransaction(SpringTransactionHooks.java:60)
  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  ...
````

Relies on the #644, #637 issues.
@ffbit
Copy link
Contributor Author

ffbit commented Dec 16, 2013

Please do not merge and keep discussion in the #637 issue's thread.

@aslakhellesoy
Copy link
Contributor

Strange - this passes on Orcale JDK and fails on OpenJDK: https://travis-ci.org/cucumber/cucumber-jvm/builds/15550646

@brasmusson
Copy link
Contributor

@aslakhellesoy No, it does not pass on Oracle JDK. On Travis the jobs on Oracle JDK does not run the examples, they instead deploy the snapshots (the Android module, or the rest of the modules, depending on the job).

@pasviegas
Copy link
Contributor

So, I did a couple of tests and It seems that when the cucumer-glue.xml is loaded there is no PlatformTransactionManager.class instance in the application context, as it is user defined, and thats why it crashes.

If you remove the SpringTransactionHooks bean from the cucumer-glue.xml and add it on the user applicationContext.xml it works.

I don't really think of an workaround, any ideas?

@brasmusson
Copy link
Contributor

To my best knowledge the situation is like this:

  • The SpringTransactionHooks class must either be declared as a bean in an xml file or have a @ContextConfiguration annotation to get the setBeanFactory method called, so it get access to a bean factory.
  • 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.
  • If the SpringTransactionHooks class is just declared as a bean in the applications context xml file, an instance of it will be instantiated when that context is loaded, but if cucumber.api.spring is part of the glue path, it will not be that instance that the @Before/After hooks will be called on, therefore those hooks will fail (see this Travis build, search for "##SpringTransactionHooks## trace printouts)
  • To get a solution that works for Cucumber-JVM v1.1.4, v1.1.5 and v1.1.6 (see the following diff):
    • Declare the SpringTransactionHooks class as a bean in the applications context xml
    • Do not have cucumber.api.spring as part of the glue path
    • Use the SpringTransactionHooks class as an autowired field in one of the applications step definitions classes, and
    • Declare @Before/After hooks, with @txn tags, in that step definition class which call startTransaction/rollBackTransaction on the autowired SpringTransactionHooks instance

BTW the see_message.feature of the spring-txn example does not, and have never, worked together with transactions, it was added during the time the spring-txn example did not include the SpringTransactionHooks class in its glue path (see this Travis build for a error log)

@mgurov
Copy link
Contributor

mgurov commented Apr 16, 2014

Would not it be nicer if hooks ( SpringTransactionHooks in this case) could share the context with the step definitions? I will give a try to look into this as much as my knowledge of cucumber-jvm and spring internals would allow.

@brasmusson
Copy link
Contributor

@mgurov I agree. Now there seems to be two very separate context. SpringTransactionHooks (and the other classes in the cucumber spring module) knows only about what is defined in classpath:cucumber/runtime/java/spring/cucumber-glue.xml, and cannot access for instance transaction managers defined in the test application and application context files.

The step definition classes on the other hand (which have the @ContextConfiguration annotation), does not know about what is defined in classpath:cucumber/runtime/java/spring/cucumber-glue.xml, for instance the cucumber-glue scope, see #600

@brasmusson
Copy link
Contributor

A "hack" to make the spring transaction support work is to let the cucumber.runtime.java.spring.SpringFactory "detect" the @ContextConfiguration annotation on the step definition classes and apply that configuration also to the SpringTransactionHooks, see #644.

@aslakhellesoy
Copy link
Contributor

If you can, please join here Mon, May 12, 3:00 PM - 4:00 PM to discuss: https://plus.google.com/u/0/events/cstdgkbvp61g7gld8lejsap4j3o

@paoloambrosio
Copy link
Member

Closing as PR #711 fixes issues #644 and #637

@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants