Details
Improvement
Resolved
Major
Description
When I add @EnableAspectJAutoProxy in my spring setup, with the bundled steps factory, my step beans can't be found.
I've done my own steps factory... it assumes the use of a specific type annotation on the step classes, to make it more clear which beans to explore, but that can be removed in a more general case.
Here it's how it looks like (WIP):
public class EnhancedSpringStepFactory extends AbstractStepsFactory { private static final Logger LOGGER = LoggerFactory .getLogger(EnhancedSpringStepFactory.class); private final ApplicationContext context; public EnhancedSpringStepFactory(ApplicationContext context, Configuration config) { super(config); this.context = context; } @Override public Object createInstanceOfType(Class<?> type) { return context.getBean(type); } @Override protected List<Class<?>> stepsTypes() { Set<Class<?>> result = new HashSet<>(); Map<String, Object> beans = context.getBeansWithAnnotation(Steps.class); for (Map.Entry<String, Object> kv : beans.entrySet()) { LOGGER.info("Possible candidate step class found in bean {}", kv.getKey()); final Class<?> target; if (AopUtils.isAopProxy(kv.getValue())) { LOGGER.info("Bean {} is an AOP proxy", kv.getKey()); target = AopProxyUtils.ultimateTargetClass(kv.getValue()); LOGGER.info("Extracted class name {} from bean {}", target.getName(), kv.getKey()); } else { LOGGER.info( "Bean {} is directly of type {} (not AOP enveloped)", kv.getKey(), kv.getValue().getClass().getName()); target = kv.getValue().getClass(); } if (hasAnnotatedMethods(target)) { if (!result.add(target)) { // TODO log sth... scoped } } else { LOGGER.warn( "Bean {} did not contain any JBehave annotated methods", kv.getKey()); } } return new LinkedList<>(result); } }
In my case it seems to work. I assume that, by returning the underlying target type in the stepTypes() method and not the proxy type, the matching logic of JBehave should work out of the box...
Do you foresee any problem with this kind of approach? if no, I can try a pull-request ...
Activity
Just be aware that you should not rely on any custom bean annotation.
All the beans in the context should be considered that have JBehave-annotated methods. So you should start from the method
@Override
protected List<Class<?>> stepsTypes() {
List<Class<?>> types = new ArrayList<Class<?>>();
for (String name : context.getBeanDefinitionNames()) {
Class<?> type = context.getType(name);
if (isAllowed(type) && hasAnnotatedMethods(type)) {
types.add(type);
}
}
return types;
}
Mauro Talevi
added a comment - Just be aware that you should not rely on any custom bean annotation.
All the beans in the context should be considered that have JBehave-annotated methods. So you should start from the method
@Override
protected List< Class <?>> stepsTypes() {
List< Class <?>> types = new ArrayList< Class <?>>();
for ( String name : context.getBeanDefinitionNames()) {
Class <?> type = context.getType(name);
if (isAllowed(type) && hasAnnotatedMethods(type)) {
types.add(type);
}
}
return types;
}
OK, I'll try to get familiar with full jbehave codebase and try a pull-request as soon as I have some spare time.
Francisco Lozano
added a comment - OK, I'll try to get familiar with full jbehave codebase and try a pull-request as soon as I have some spare time. Please take a look at https://github.com/jbehave/jbehave-core/pull/67 .
However:
test `shouldBuildCandidateStepsFromAnnotationsUsingSpring` still fails, and to be honest I think it's due to the test being wrong:
The configuration class used in this test looks like:
@Configure()
@UsingSpring(resources = { "org/jbehave/core/configuration/spring/configuration.xml",
"org/jbehave/core/steps/spring/steps.xml", "org/jbehave/core/steps/spring/steps-with-dependency.xml" })
private static class AnnotatedUsingSpring {
}
which is mixing both steps.xml and steps-with-dependency.xml. However, FooStepsWithDependency inherits from FooSteps, a condition I think would not work (because of repeated definitions of the same steps in two step classes).
Failure of that test is like:
java.lang.AssertionError:
Expected: an instance of org.jbehave.core.steps.spring.SpringStepsFactoryBehaviour$FooStepsWithDependency
but: <org.jbehave.core.steps.spring.SpringStepsFactoryBehaviour$FooSteps@289d1c02> is a org.jbehave.core.steps.spring.SpringStepsFactoryBehaviour$FooSteps
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:8)
at org.jbehave.core.configuration.spring.SpringAnnotationBuilderBehaviour.assertThatStepsInstancesAre(SpringAnnotationBuilderBehaviour.java:141)
at org.jbehave.core.configuration.spring.SpringAnnotationBuilderBehaviour.shouldBuildCandidateStepsFromAnnotationsUsingSpring(SpringAnnotationBuilderBehaviour.java:111)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:483)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
It depends purely on the order the getBeanDefinitionNames() is returning the two beans (fooStep and fooStepWithDependency).
I think it'd be better to explicitly fail on this by using context.getBean(Class<?> type), which would fail because more than one bean would match that type I think:
public Object createInstanceOfType(Class<?> type) { try { return context.getBean(type); } catch (NoSuchBeanDefinitionException e) { throw new StepsInstanceNotFound(type, this); } }
Francisco Lozano
added a comment - - edited Please take a look at https://github.com/jbehave/jbehave-core/pull/67 .
However:
test `shouldBuildCandidateStepsFromAnnotationsUsingSpring` still fails, and to be honest I think it's due to the test being wrong:
The configuration class used in this test looks like:
@Configure()
@UsingSpring(resources = { "org/jbehave/core/configuration/spring/configuration.xml" ,
"org/jbehave/core/steps/spring/steps.xml" , "org/jbehave/core/steps/spring/steps-with-dependency.xml" })
private static class AnnotatedUsingSpring {
}
which is mixing both steps.xml and steps-with-dependency.xml. However, FooStepsWithDependency inherits from FooSteps, a condition I think would not work (because of repeated definitions of the same steps in two step classes).
Failure of that test is like:
java.lang.AssertionError:
Expected: an instance of org.jbehave.core.steps.spring.SpringStepsFactoryBehaviour$FooStepsWithDependency
but: <org.jbehave.core.steps.spring.SpringStepsFactoryBehaviour$FooSteps@289d1c02> is a org.jbehave.core.steps.spring.SpringStepsFactoryBehaviour$FooSteps
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:8)
at org.jbehave.core.configuration.spring.SpringAnnotationBuilderBehaviour.assertThatStepsInstancesAre(SpringAnnotationBuilderBehaviour.java:141)
at org.jbehave.core.configuration.spring.SpringAnnotationBuilderBehaviour.shouldBuildCandidateStepsFromAnnotationsUsingSpring(SpringAnnotationBuilderBehaviour.java:111)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:483)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
It depends purely on the order the getBeanDefinitionNames() is returning the two beans (fooStep and fooStepWithDependency).
I think it'd be better to explicitly fail on this by using context.getBean(Class<?> type), which would fail because more than one bean would match that type I think:
public Object createInstanceOfType( Class <?> type) {
try {
return context.getBean(type);
} catch (NoSuchBeanDefinitionException e) {
throw new StepsInstanceNotFound(type, this );
}
}
Thanks for the pull request. It looks good at first sight. Will try to pull shortly.
As for the test, you are right that it should not rely on the order.
Mauro Talevi
added a comment - Thanks for the pull request. It looks good at first sight. Will try to pull shortly.
As for the test, you are right that it should not rely on the order. Thanks!
The test also contains explicit exclusion for steps declaration in interfaces. For example,
public interface FooBarSteps { @Given("whatever bla bla") void givenWhateverBlaBla(); } public class FooBarStepsImpl { @Override public void givenWhateverBlaBla() { } }
it ensures (same as current behavior) that those are not exposed by the steps factory.
However, I think it'd be great to support them. It would allow JDK Proxies for steps, and would allow separating step interface construction from actual step implementation.
One example use-case would be making it possible to test, with the same stories, two "client libraries" with the same functionality but different APIs, by reimplementing the step classes using each library's API.
I think the StepsFactory changes needed are simple, but i assume also some other changes would be needed somewhere else (and I haven't explored enough yet to identify).
Francisco Lozano
added a comment - - edited Thanks!
The test also contains explicit exclusion for steps declaration in interfaces. For example,
public interface FooBarSteps {
@Given( "whatever bla bla" )
void givenWhateverBlaBla();
}
public class FooBarStepsImpl {
@Override
public void givenWhateverBlaBla() {
}
}
it ensures (same as current behavior) that those are not exposed by the steps factory.
However, I think it'd be great to support them. It would allow JDK Proxies for steps, and would allow separating step interface construction from actual step implementation.
One example use-case would be making it possible to test, with the same stories, two "client libraries" with the same functionality but different APIs, by reimplementing the step classes using each library's API.
I think the StepsFactory changes needed are simple, but i assume also some other changes would be needed somewhere else (and I haven't explored enough yet to identify).
Please raise a different issue for the proxy behaviour
Mauro Talevi
added a comment - Please raise a different issue for the proxy behaviour Mauro Talevi
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Affects Version/s | 4.0 [ 18486 ] |
Mauro Talevi
made changes -
| Resolution | Fixed [ 1 ] | |
| Fix Version/s | 3.9.5 [ 20598 ] | |
| Status | Open [ 1 ] | Resolved [ 5 ] |
I see no reason to have a separate factory. AOP support could be added to the existing SpringStepsFactory.
We can make it optional via a flag which is injected in the constructor.
If you could provide a pull request with the appropriate changes, and a test case to verify it's functionality, it'd be greatly appreciated.