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

Dropwizard testing module for JUnit 5 #2166

Merged
merged 1 commit into from Nov 21, 2017

Conversation

Projects
None yet
5 participants
@AnDyXX
Contributor

AnDyXX commented Oct 3, 2017

This PR is proposition of Dropwizard testing module with use of JUnit 5 (Issue #1944). Contains following:

  • code of original test rules was changed to support executing them from JUnit 5 extension,
  • JUnit 5 extension class that executes classes from point above,
  • code of original tests modified to use JUnit 5.

Example of use:


@ExtendWith(DropwizardExtensionsSupport.class)
public class EpisodesResourceTest {

    public final ResourceExtension resources = ResourceExtension.builder()
            .addResource(new UsersResource(usersService))
            .build();
			
@nickbabcock

This comment has been minimized.

Contributor

nickbabcock commented Oct 3, 2017

Hey @AnDyXX, thanks for kicking this off! Much appreciated.

I know that there is a lot of copied code from testing module that could be extracted to common testing library.

Is it possible to achieve backwards compatibility for the current junit4 users with a common core module? If not, we'd have to wait for something like a major version bump to introduce this change.

I'm guessing it's not possible to create the dropwizard-testing-junit5 as a tacked on module to dropwizard-testing (eg. users poms would look like)

        <dependency>
            <groupId>io.dropwizard</groupId>
            <artifactId>dropwizard-testing</artifactId>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>io.dropwizard</groupId>
            <artifactId>dropwizard-testing-junit5</artifactId>
            <scope>test</scope>
        </dependency>

As that might be the most preferable 🤔

@AnDyXX

This comment has been minimized.

Contributor

AnDyXX commented Oct 3, 2017

Is it possible to achieve backwards compatibility for the current junit4 users with a common core module?

Yep, just my first impuls was to not change anything - rather add new functionality to separate module.

@AnDyXX AnDyXX force-pushed the AnDyXX:junit5 branch from 54bc9fb to 314a9a5 Oct 3, 2017

@coveralls

This comment has been minimized.

coveralls commented Oct 3, 2017

Coverage Status

Coverage decreased (-3.3%) to 82.114% when pulling 314a9a5 on AnDyXX:junit5 into e8f7f6c on dropwizard:master.

@AnDyXX

This comment has been minimized.

Contributor

AnDyXX commented Oct 3, 2017

I changed code so JUnit4 and JUnit5 code is in the dropwizard-testing module.
Changed junit 4 dependency from junit to junit-vintage-engine.
Common code extracted.

@AnDyXX AnDyXX changed the title from Dropwizard testing module for JUnit 5 [WIP] to Dropwizard testing module for JUnit 5 Oct 3, 2017

@coveralls

This comment has been minimized.

coveralls commented Oct 3, 2017

Coverage Status

Coverage decreased (-1.7%) to 83.739% when pulling 5b34ad1 on AnDyXX:junit5 into e8f7f6c on dropwizard:master.

@joschi

This comment has been minimized.

Member

joschi commented Oct 3, 2017

Changed junit 4 dependency from junit to junit-vintage-engine.

Unfortunately that makes it incompatible with JUnit and would always pull in JUnit 5 and its vintage engine.

How about making the dependencies for JUnit 4 and 5 optional?
https://maven.apache.org/guides/introduction/introduction-to-optional-and-excludes-dependencies.html

@coveralls

This comment has been minimized.

coveralls commented Oct 4, 2017

Coverage Status

Coverage decreased (-2.3%) to 83.124% when pulling 0e318ca on AnDyXX:junit5 into e8f7f6c on dropwizard:master.

@coveralls

This comment has been minimized.

coveralls commented Oct 4, 2017

Coverage Status

Coverage decreased (-1.2%) to 84.237% when pulling 06f4d97 on AnDyXX:junit5 into e8f7f6c on dropwizard:master.

@AnDyXX

This comment has been minimized.

Contributor

AnDyXX commented Oct 5, 2017

Ok I made JUnit 5 optional and updated restored comments.

@coveralls

This comment has been minimized.

coveralls commented Oct 5, 2017

Coverage Status

Coverage decreased (-1.2%) to 84.237% when pulling 3fbf4d5 on AnDyXX:junit5 into e8f7f6c on dropwizard:master.

@joschi joschi added this to the 2.0.0 milestone Oct 5, 2017

@arteam arteam modified the milestones: 2.0.0, 1.2.1 Oct 26, 2017

@arteam arteam added the feature label Oct 26, 2017

@joschi

This comment has been minimized.

Member

joschi commented Nov 13, 2017

@AnDyXX Could you please rebase this PR on current master?

@AnDyXX AnDyXX force-pushed the AnDyXX:junit5 branch from 3fbf4d5 to 0d3f7d9 Nov 14, 2017

@@ -34,7 +31,7 @@ long getId() {
}
String getDescription() {
return description;
return requireNonNull(description);

This comment has been minimized.

@joschi

joschi Nov 15, 2017

Member

What is that for?

Throwing a NullPointerException in a getter is very bad practice.

This comment has been minimized.

@AnDyXX

AnDyXX Nov 15, 2017

Contributor

Actually this one comes from this class: https://github.com/dropwizard/dropwizard/blob/master/dropwizard-testing/src/test/java/io/dropwizard/testing/junit/TestEntity.java

It is used in tests there: io.dropwizard.testing.junit5.DAOTestExtensionTest#rollsBackTransaction and similar test is there: io.dropwizard.testing.junit.DAOTestRuleTest#rollsBackTransaction

import javax.persistence.Table;
import javax.validation.constraints.NotNull;
import javax.annotation.Nullable;
import javax.persistence.*;

This comment has been minimized.

@joschi

joschi Nov 15, 2017

Member

Please don't use wildcard imports but name all imports explicitly.

import javax.persistence.Id;
import javax.persistence.Table;
import javax.validation.constraints.NotNull;
import javax.persistence.*;

This comment has been minimized.

@joschi

joschi Nov 15, 2017

Member

Please don't use wildcard imports but name all imports explicitly.

This comment has been minimized.

@AnDyXX

AnDyXX Nov 15, 2017

Contributor

I addressed your comments, have to look at other failing issues.

import org.hibernate.cfg.Configuration;
import org.hibernate.context.internal.ManagedSessionContext;
import java.util.*;

This comment has been minimized.

@joschi

joschi Nov 15, 2017

Member

Please don't use wildcard imports but name all imports explicitly.

@@ -0,0 +1,94 @@
package io.dropwizard.testing.junit5;
import org.junit.jupiter.api.extension.*;

This comment has been minimized.

@joschi

joschi Nov 15, 2017

Member

Please don't use wildcard imports but name all imports explicitly.

import static io.dropwizard.testing.ResourceHelpers.resourceFilePath;
import static org.hamcrest.core.Is.is;
import static org.hamcrest.MatcherAssert.*;

This comment has been minimized.

@joschi

joschi Nov 15, 2017

Member

Please don't use wildcard imports but name all imports explicitly.

import java.util.Collections;
import static io.dropwizard.testing.ResourceHelpers.resourceFilePath;
import static javax.ws.rs.core.HttpHeaders.*;

This comment has been minimized.

@joschi

joschi Nov 15, 2017

Member

Please don't use wildcard imports but name all imports explicitly.

import javax.ws.rs.ext.ExceptionMapper;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.*;

This comment has been minimized.

@joschi

joschi Nov 15, 2017

Member

Please don't use wildcard imports but name all imports explicitly.

package io.dropwizard.testing.junit5;
import javax.annotation.Nullable;
import javax.persistence.*;

This comment has been minimized.

@joschi

joschi Nov 15, 2017

Member

Please don't use wildcard imports but name all imports explicitly.

@AnDyXX AnDyXX force-pushed the AnDyXX:junit5 branch 2 times, most recently from ee14087 to 25ab092 Nov 15, 2017

@coveralls

This comment has been minimized.

coveralls commented Nov 17, 2017

Coverage Status

Coverage decreased (-0.3%) to 86.908% when pulling 158e5d9 on AnDyXX:junit5 into 4a82069 on dropwizard:master.

@AnDyXX

This comment has been minimized.

Contributor

AnDyXX commented Nov 20, 2017

I had to downgrade maven-surefire-plugin to 2.19.1 version to make oit work with JUnit5. codeclimate — 42 issues to fix are mostly because of tests for JUnit5 that are copied from junit 4 counterpart. Any hints on this ones?

@arteam

This comment has been minimized.

Member

arteam commented Nov 20, 2017

I've manually marked warnings in Codeclimate as false positives.

Andrzej Kurzeja Andrzej Kurzeja
Add support for JUnit 5.
* code of original test rules was changed to support executing them as JUnit 5 extensions,
* maven-surefire-plugin version downgraded to 2.19.1 to work with JUnit 5,
* code of original tests copied and modified to use JUnit 5.

@AnDyXX AnDyXX force-pushed the AnDyXX:junit5 branch from 158e5d9 to 6752eb2 Nov 21, 2017

@AnDyXX

This comment has been minimized.

Contributor

AnDyXX commented Nov 21, 2017

PR code rebased.

@joschi

joschi approved these changes Nov 21, 2017

@arteam arteam merged commit 450e23e into dropwizard:master Nov 21, 2017

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codeclimate Approved by Artem Prigoda.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@arteam

This comment has been minimized.

Member

arteam commented Nov 21, 2017

Thank you for the contribution!

sankate pushed a commit to sankate/dropwizard that referenced this pull request Nov 21, 2017

Add support for JUnit 5. (dropwizard#2166)
* code of original test rules was changed to support executing them as JUnit 5 extensions,
* maven-surefire-plugin version downgraded to 2.19.1 to work with JUnit 5,
* code of original tests copied and modified to use JUnit 5.

aaanders added a commit to aaanders/dropwizard that referenced this pull request Sep 20, 2018

Add support for JUnit 5. (dropwizard#2166)
* code of original test rules was changed to support executing them as JUnit 5 extensions,
* maven-surefire-plugin version downgraded to 2.19.1 to work with JUnit 5,
* code of original tests copied and modified to use JUnit 5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment