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

Categories + BeforeClass #93

Closed
psenger opened this issue Mar 11, 2010 · 20 comments
Closed

Categories + BeforeClass #93

psenger opened this issue Mar 11, 2010 · 20 comments

Comments

@psenger
Copy link

psenger commented Mar 11, 2010

It would be nice if we could govern the test harness state with categories ( refer to the example ). However, it would appear the Categories Test Runner does not pick up the Before Class annotation or at least can not discern between the presences of two. Is this a bug Mr. Beck? Thanks for you support. Best Regards, Philip A Senger

import org.junit.runner.;
import org.junit.experimental.categories.
;
import org.junit.runners.*;
@RunWith(Categories.class)
@Categories.IncludeCategory(Development.class)
@Suite.SuiteClasses({MytestClass.class})
public class Development
{

}

import org.junit.runner.;
import org.junit.experimental.categories.
;
import org.junit.runners.*;
@RunWith(Categories.class)
@Categories.IncludeCategory(Stage.class)
@Suite.SuiteClasses({MytestClass.class})
public class Stage
{

}

import org.junit.experimental.categories.;
import org.junit.
;
public class MytestClass
{
private static String color = null;
/**

  • Looks like the BeforeClass is not subject to the Categories strategy.
    /
    @category({Development.class})
    @BeforeClass
    public static void setupAllTestsDev()
    {
    color = "red";
    }
    /
  • Looks like the BeforeClass is not subject to the Categories strategy.
    **/
    @category({Stage.class})
    @BeforeClass
    public static void setupAllTestsStage()
    {
    color = "blue";
    }
    @category({Stage.class})
    @test
    public void testStageColor () {
    Assert.assertEquals("The color is not blue", "blue", color );
    }
    @category({Development.class})
    @test
    public void testDevColor () {
    Assert.assertEquals("The color is not red", "red", color );
    }
    }
@dsaff
Copy link
Member

dsaff commented Mar 4, 2011

You are right--at this point, a Category annotation on a BeforeClass has no effect whatsoever. We should probably either honor the annotation, or throw a validation error if it is attempted.

@dsaff
Copy link
Member

dsaff commented May 10, 2013

The validation error would be low-hanging fruit if anyone wanted to help.

@codingricky
Copy link

I'd be happy to have a look, any hints on where to start?

@dsaff
Copy link
Member

dsaff commented May 13, 2013

This may end up being more involved than I thought. We don't want BlockJUnit4ClassRunner to know specifically about Categories. So to make this happen, we may want some kind of "meta-annotation" that can be put on an annotation like @category, explaining in what situations it is valid. Something like:

@Retention(RetentionPolicy.RUNTIME)
@Inherited
@CannotCombine({BeforeClass.class, AfterClass.class, Before.class, After.class})
public @interface Category {
    Class<?>[] value();
}

Then ParentRunner or BlockJUnit4ClassRunner's validation code would need to be enhanced to recognize and enforce this @CannotCombine annotation.

@codingricky
Copy link

So the runner needs to check whether the test has an annotation that has the @CannotCombine annotation on it (for e.g, in your case above, the @category annotation). If it does, then it needs to check whether the other annotations in the @CannotCombine list have been applied as well (this could be targeting a method/class depending on the @target annotation of the class). If yes, throw an Exception.

Is that the gist of it? Is there anything that does something similar at the moment?

Thanks!

@dsaff
Copy link
Member

dsaff commented May 14, 2013

No, I don't think anything else does anything like that.

Another, potentially even better, possibility would be to have a @validator annotation that gives the name of a class that can be instantiated to validate a usage of an annotation. So this would look like

@Retention(RetentionPolicy.RUNTIME)
@Inherited
@Validator(CategoryValidator.class)
public @interface Category {
    Class<?>[] value();
}

CategoryValidator would have to have a no-argument constructor, and implement an interface something like this:

interface AnnotationValidator {
  void validateAnnotatedClass(Class<?> type, List<Throwable> errors);
  void validateAnnotatedField(Field field,  List<Throwable> errors);
  void validateAnnotatedMethod(Method method, List<Throwable> errors);
}

Actually, I think this could be a really nice framework. Thoughts?

@codingricky
Copy link

Ok I think I understand where you are going with this. So somewhere (in the BlockJUnit4ClassRunner or ParentRunner) you need to check that an annotation (like @category) has the @validator annotation on it, and if it does, then create the Validator specified and invoke it.

What level would you do this checking? Would you check that any annotation on the TestClass has this @validator annotation on it?

Thanks and sorry for the beginner questions!

@dsaff
Copy link
Member

dsaff commented May 15, 2013

There are no bad questions. :-)

My thought is: create a static method somewhere that would check all of the annotations on a class to invoke any @Validators attached to any of the annotations. Then call that from ParentRunner#collectInitializationErrors.

Thanks for looking into this!

@codingricky
Copy link

When invoking the validator, going by the interface you had, would you invoke it once for each fields and each method on the test class? Is this ok?

@dsaff
Copy link
Member

dsaff commented May 17, 2013

My thought was that we'd look at each class, method, and field, look at each annotation, and find if it has a Validator registered. If so, then the Validator would be called only on the structure on which it was defined. Make sense?

@codingricky
Copy link

Ok so just testing my understanding and summarising everything.

  • Define the interface annotation AnnotationValidator
  • Define the @Validation annotation
  • Implement the CategoryValidator
  • Attach the category validator to the @Category annotation
  • In the parent runner, call a static method to check each class, method and field to look for the validator annotation, and if it exists, create an instance of the validator and invoke it on the structure on which it was defined.

For e.g., the @Category annotation would have a @Validator of type CategoryValidator. When checking a class with the @Category annotation, the categoryValidator.validateAnnotatedClassmethod would be invoked passing in the test class. This particular validator would check for the presence of any of the following annotations BeforeClass.class, AfterClass.class, Before.class, After.class on any method of the class. If any such annotations existed, an error would be added to the incoming list.

Is that about correct?

Thanks again!

@dsaff
Copy link
Member

dsaff commented May 20, 2013

Really close. A couple nits:

  1. I don't think the method on ParentRunner has to be static, but I might not have thought through the implications.
  2. The actual bug we're trying to prevent is a user who annotates a method with both @BeforeClass and @category, so it would be the validateMethod that would trigger, although there's probably other validation checks we could consider.

Thanks again!

@codingricky
Copy link

  1. I thought you said earlier to put a static method somewhere that ParentRunner would invoke. I was thinking of putting this into another class or where you thinking in the ParentRunner?
  2. Understood. So this check would be in validateAnnotatedMethod of the CategoryValidator

@codingricky
Copy link

Is there an easy way to look through each annotation of each class, field and method? Currently the TestCase class has this information but seems to only have methods to return annotated fields and methods if you pass in an annotation first. That is, you can't just list all the annotated fields/methods of a TestCase.

Within the ParentRunner, do I need to call getTestClass().getJavaClass().getMethods() myself? or is there another way to get this information that I'm missing?

Thanks!

@dsaff
Copy link
Member

dsaff commented May 22, 2013

I'd add an extra method or two to TestClass that would expose all the annotations used as keys on fMethodsForAnnotations and fFieldsForAnnotations. Annotation hunting is pretty expensive, so taking advantage of the caching we've already done is key.

@codingricky
Copy link

Just put in a pull request #684

Let me know your thoughts, I'm happy to revise/change according to feedback. I thought I'd code up what I was thinking to see if I was on the mark or not.

Thanks!

dsaff pushed a commit that referenced this issue Oct 24, 2013
Add AnnotationValidator, and validation checks for Category (fix for issue #93)
@jvz
Copy link

jvz commented Dec 29, 2013

So is this bug solved now? The pull request is merged already.

@kcooney
Copy link
Member

kcooney commented Jan 7, 2014

@jvz Support for validations was added. I'm not sure if @codingricky added a validation check for this particular problem.

@dsaff
Copy link
Member

dsaff commented Jan 7, 2014

Agreed, this is now fixed!

@dsaff dsaff closed this as completed Jan 7, 2014
test-git-x-modules-app bot pushed a commit to vs/junit4 that referenced this issue Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants