Support for annotated JUnit @Category

Why not allow the selection of any annotation ?

As long as patches contains test coverage and they don't seem totally "off" from a JUnit perspective I'm all for flexible solutions

I've attached a patch with integration tests that allows for test filter based on arbitrary class level annotations in addition to using the JUnit Category annotation method.

Attaching a second patch that builds on the 1st:

Most class level annotations on a test class are now able to be used within surefire's groups/excludedGroups configuration properties. Annotations are added recursively so that annotations of annotations are supported. Annotations within java.lang.annotation and org.junit.experimental.categories.Category are excluded.

Example of how this works:

@Inherited
@Retention(RetentionPolicy.RUNTIME)
@Target(value = {ElementType.ANNOTATION_TYPE, ElementType.TYPE})
@interface AnnotationParent {}

@AnnotationParent
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@interface AnnotationA {}

@AnnotationParent
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@interface AnnotationB {}

// No parent annotation
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@interface AnnotationC {}

@AnnotationA
class ATest {}

@AnnotationB
class BTest {}

@AnnotationC
class CTest{}

// No Annottion
class NTest{}

Some possible includes/excludes using surefire:

<!-- Run A,B only -->
<groups>my.pkg.AnnotationA, my.pkg.AnnotationB</groups>
<!-- or -->
<groups>my.pkg.AnnotationParent</groups>

<!-- Run C,N only -->
<excludedGroups>my.pkg.AnnotationA, my.pkg.AnnotationB</excludedGroups>
<!-- or -->
<excludedGroups>my.pkg.AnnotationParent</excludedGroups>

Cool patch, I like the feature already - having to add Interfaces in order to put them into a @Category-annotation always feels weird to me, too.

If you'd like to make that thing sound and complete, we already have some documentation on grouping on the examples-pages that would also need some extensions. Source is in maven-surefire-plugin/src/site/apt/examples/junit.apt.vm. But please mention that these annotations work on class-level only, and that it's a Surefire addition to what JUnit offers (for those people executing their tests in an IDE without Surefire, but some IDE-specific JUnit integration).

If you don't have the time to append the docs, I will do it after we applied your patches.

I am slightly sceptical to this patch overall since we're going a bit far into "being" a test framework. I'd like to see this discussed within the junit project before creating all kinds of surefire specific junit extensions.

I know this has been discussed within JUnit. At the time it seemed to be like they were considering doing /more/ of the annotation processing within the framework and less within the runner. I believe there's already more options for doing annotation processing within junit (which we dont use)

Please start this discussion on the junit mailing list and we'll take it from there. Sorry if I appear to have changed my mind; it's just a slight detour. We'll get there in the end

I don't see why or how this functionality would be part of JUnit. This patch is only concerned with selecting a set of test classes to run. JUnit provides APIs for this to be done; i.e. Runners and Filters. Surefire is already doing filtering based on the filename and JUnit category annotations so why is this any different?

If you don't want this code in surefire what about adding an feature to allow for the inject of custom junit Filters into the classpath scanning process?

You may very well be right; I just seem to recall a quite recent discussion on the Junit mailing list about extending the scope of junit to be more involved in filtering.

I'm not saying I don't want this patch as such; I just want to discuss it a little in context of junit first. The thing about our current filtering logic is that it's probably not very optimal (at least I tend to think it's a mess every time I try to change it - I can say that safely since I wrote most of it The concerns in the discussion on the junit list was that all the runners implement slightly different scanning logic, and some of it should probably be moved out of the various runners and into junit.

I think the patch is cool; I want this as much as you do. Let's just run it past the junit folks first.