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

Updated CategoryFilter to prevent NPE with Cucumber and JUnit Categories #1511

Merged

Conversation

andyrbell
Copy link
Contributor

@andyrbell andyrbell commented Mar 5, 2017

Fix for #1510

  • NullPointerException was being thrown if JUnit Categories were used in a project containing Cucumber tests.
  • Ensure CategoryFilter does not call createSuiteDescription if the description test class is null
  • Added integration test to reproduce the error

This bug was originally reported by @krosenvold on the Gradle Forum.

Any of the checked boxes below indicate that I took action:

For all non-trivial changes that modify the behavior or public API:

  • Before submitting a pull request, I started a discussion on the Gradle developer list,
    the forum or can reference a JIRA issue.
  • I considered writing a design document. A design document can be
    brief but explains the use case or problem you are trying to solve,
    touches on the planned implementation approach as well as the test cases
    that verify the behavior. Optimally, design documents should be submitted
    as a separate pull request. Samples
    can be found in the Gradle GitHub repository. Please let us know if you need help with
    creating the design document. We are happy to help!
  • The pull request contains an appropriate level of unit and integration
    test coverage to verify the behavior. Before submitting the pull request
    I ran a build on my local machine via the command
    ./gradlew quickCheck <impacted-subproject>:check.
  • The pull request updates the Gradle documentation like user guide,
    DSL reference and Javadocs where applicable.

- NullPointerException was being thrown if JUnit Categories were used in
  a project containing Cucumber tests.
- Ensure CategoryFilter does not call createSuiteDescription if the
  description test class is null
- Added integration test to reproduce the error

test {
useJUnit {
excludeCategories 'CategoryA'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the relevance of this category if it is excluded anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CategoryFilter, the source of the bug, is only invoked if a category inclusion / exclusion is specified here. It doesn't matter what is excluded. In fact, no test is even annotated with this category.

public class DescriptionWithNullClassTest {

@Test
public void someTest() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't need to throw an exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


@Test
public void someTest() throws Exception {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the unnecessary new lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -64,4 +64,16 @@ public class JUnitCategoriesIntegrationSpec extends AbstractIntegrationSpec {
def result = new DefaultTestExecutionResult(testDirectory)
result.testClass("org.gradle.SomeTest").assertTestFailed("initializationError", startsWith("org.gradle.api.GradleException: JUnit Categories defined but declared JUnit version does not support Categories."))
}

def supportsCategoriesAndNullTestClassDescription() {
when:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please a new line between when and then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

run "test"
then:
":test" in nonSkippedTasks
and:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove the last and statement. It's OK to just have this under then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@bmuschko bmuschko merged commit 40fe9ad into gradle:master Apr 12, 2017
@bmuschko bmuschko added this to the 4.0 RC1 milestone Apr 12, 2017
@bmuschko
Copy link
Contributor

Thanks for the pull request. It has been merged!

@andyrbell
Copy link
Contributor Author

Awesome, thanks!

@bmuschko
Copy link
Contributor

Sure thing. Sorry about the long wait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants