Remove dependency on javassist by using standard JDK annotation functionality

Brian, JDK don't allow you to merge arrays attributes if you need extend your Embedders and reuse your annotated steps classes...

It was the mainly reason that I've choosed to use javassist.

cheers

Cristiano,

Thanks for the comment - good to know the "why".

One can walk the class hierarchy tree and do that work (just like walking interfaces and superclasses to look for un-inherited annotations). That is no different than how Spring-Test's @ContextConfiguration works. But the actual reading of the annotation information doesn't require javassist.

Do we always combine arrays for every annotation or do we need a boolean parameter on some of those methods? What about interfaces?

Brian

Looked over the annotations and the original AnnotationFinder.

It looks like we are only talking about Type annotations here, since AnnotationFinder only looks at the class definitions for visible and invisible annotations. Also a scalar (single-valued) property is always overriden by the current logic (and there is no such thing as a missing property on an annotation - it might have a default value - but it has to be there).

So we only need to design for multi-valued properties in Type annotations. That list includes the following:

@UsingSteps - instances
@UsingGuice - modules
@UsingPico - modules
@UsingSpring - resources

All of these have use cases where one would both want to and not want to inherit the values from the super class(es). Fortunately, we only have 1 multi-valued property in each one.

Spring deals with this situation by adding another property to @ContextConfiguration by having a String[] locations property (what we call resources, with a default of an empty array) and then a boolean inheritLocations property with a default value of true. The code that process this routine walks up the class hierarchy and adds locations to the ultimate array. Spring does this in the same way that AnnotationFinder does. Both use List instead of Set - since the use of the List takes into account duplicates (though, technically, Set would be better).

I'm not sure if Pico or Guice have annotations that create containers so I'm not sure what those do.

I would recommend the same pattern.

@UsingSteps - instances and inheritInstances
@UsingGuice - modules and inheritModules
@UsingPico - modules and inheritModules
@UsingSpring - resources and inheritResources

The problem is now that there is no known property - we would have to code for realizing that the value is an array and then build the name of the inheriting property and check that up the class hierarchy.

But I also think that the existing code, which always inherits/appends isn't the best option either. There are use cases where one wants to turn that off.

What do people think? I'd prefer to have some buy-in before coding.

Also, do we want to extend the use of @Using* annotations to interfaces?

Attaching patch files (finally learned git format-patch) for all changes including the latest which is the implementation and test of the above change.

Not sure how great of a "patch" these are but you should be able to apply these...

Actually, I'm having trouble applying them. My patch bin does not understand them.

Could you simply try recreating them from command line using git diff:

git diff > <name>.patch

Thanks

Applied patches 1 and 2 (after battling with issues with trailing whitespace).

Not yet applied 3: I agree with making the inheritance behaviour configurable, but would like to avoid proliferation of boolean properties in multiple annotations. Rather, we could introduce another separate annotation, say @UsingInheritance, that could be used independently of the method of composition used. WDYT?

Resolving this issue, as the dependency on javassist as been removed.

Inheritance issue now tracked by JBEHAVE-329.