-
Notifications
You must be signed in to change notification settings - Fork 285
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
Recognize JUnit 5's @BeforeAll and @BeforeEach as initializers #73
Comments
@michaelhixson looks like a good change. Could you submit a PR with the change? Tests not required. |
@msridhar I'm not comfortable with signing the CLA, so I closed the PR. Sorry for the noise. I still think the change is a good idea though. |
@michaelhixson can you contact me 1-1 (email on my website, linked from my profile)? Wanted to follow up briefly on the CLA issues. Thanks! |
msridhar
added a commit
that referenced
this issue
Dec 14, 2017
Support @beforeeach and @BeforeAll as initializer annotations, and @Inject and @lazyinit as excluded field annotations. Fixes #73, #64
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
NullAway should recognize JUnit 5's
@BeforeAll
and@BeforeEach
as initializer annotations by default. Currently it recognizes JUnit 4's@Before
and@BeforeClass
, but in JUnit 5 these were renamed:org.junit.jupiter.api.BeforeAll
org.junit.jupiter.api.BeforeEach
Since you provided the
CustomInitializerAnnotations
option, users can work around this by passing an argument to javac:...but I think it makes sense to support this by default so users don't have to do that.
Resolving this issue should be a matter of changing these lines:
NullAway/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java
Lines 60 to 63 in c48edad
to this:
Adding test cases could be awkward since you'd need to bring in a JUnit 5 dependency probably? Or you could not add tests.
The text was updated successfully, but these errors were encountered: