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

Disallow annotating a non-static method with @BeforeClass #25

Open
cushon opened this issue Oct 31, 2014 · 11 comments
Open

Disallow annotating a non-static method with @BeforeClass #25

cushon opened this issue Oct 31, 2014 · 11 comments

Comments

@cushon
Copy link
Collaborator

cushon commented Oct 31, 2014

Original issue created by eaftan@google.com on 2012-07-16 at 07:18 PM


The JUnit @BeforeClass annotation should only be applied to a public static void no-arg method, otherwise it throws a runtime error. We should detect this at compile time.

There may also be other JUnit-specific checks we can write. For example, JUnit doesn't run tests that are not marked public. This can cause someone to mistakenly think that their newly created tests are runnign when they're actually not. Some other reason tests weren't being run:

  • The test class did not extend TestCase
  • The test method wasn't public
  • The test method didn't start with "test" (e.g. tesSomething)
  • The test method took a parameter
@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by kcooney@google.com on 2012-07-16 at 07:51 PM


Note that JUnit4-style tests should not extend TestCase, but perhaps that should be a warning, not an error.

Note that JUnit4-style tests do not need to have method names that start with "test", but they do need to be annotated with @Test. It would be great to detect at compile time methods in JUnit4-style tests that are 1) public, 2) contain no arguments, 3) have a method name starting with "test", and 4) are not annotated with @Test. JUnit will not fail a test that contains a method like this, while it will fail a test that uses @BeforeClass in a non-public or non-static method.

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by alexeagle@google.com on 2012-07-16 at 10:43 PM


Issue #24 has been merged into this issue.

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by eaftan@google.com on 2012-08-27 at 06:01 PM


Additional cases to check for from yoavtz:

  1. A class ending with the word Test that is not annotated with @RunWith
  2. A method starting with the word "test" that is not annotated with @Test (possibly only in classes annotated with @RunWith(JUnit4.class)
  3. A method called setup or setUp that is not annotated with @Before
  4. A method called teardown or tearDown that is not annotated with @After

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by eaftan@google.com on 2013-02-11 at 09:38 PM


(No comment entered for this change.)


Status: Accepted
Labels: -Priority-Medium, Priority-High

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by eaftan@google.com on 2013-03-18 at 05:43 PM


A JUnit 3 case from jwray:

  1. A method starting with the word "Test" (uppercase), which doesn't run because JUnit 3 expects test methods to start with a lowercase "test". Note that standard Java style expects methods to start with a lowercase letter anyway.

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by eaftan@google.com on 2013-03-18 at 11:58 PM


(No comment entered for this change.)


Owner: eaftan@google.com

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by eaftan@google.com on 2013-03-21 at 12:04 AM


Another case from pepstein:

You have a JUnit 3 test (no @RunWith) but use @Test annotations on your methods. Those tests won't run if they don't match the JUnit 3 naming conventions.

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by eaftan@google.com on 2013-04-09 at 05:35 PM


I committed a check for JUnit4-style tests that do not have an @Test annotation. Will be released in v0.9.13.

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by kcooney@google.com on 2013-08-19 at 10:38 PM


It's perfectly valid to have a class ending with the word Test that is not annotated with @RunWith and does not extend junit.framework.TestCase. JUnit4 does not require that test classes have a @RunWith annotation

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by eaftan@google.com on 2013-08-19 at 11:09 PM


kcooney, acknowledged.

@cushon
Copy link
Collaborator Author

cushon commented Oct 31, 2014

Original comment posted by kevinb@google.com on 2013-10-01 at 04:40 PM


Just noting that if BeforeClass.java could have @RequiredModifiers({PUBLIC, STATIC}) added to it, that would take care of this. Of course that's probably not possible.

cushon pushed a commit to cushon/error-prone that referenced this issue Oct 31, 2014
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

1 participant