Skip to content

Commit

Permalink
Merge pull request IQSS#9796 from IQSS/9782-migrate-junit5
Browse files Browse the repository at this point in the history 
Migrate all tests to JUnit5
  • Loading branch information
@kcondon
kcondon authored Aug 23, 2023
2 parents bf04fda + 494624b commit 497c578
Show file tree
Hide file tree
Showing 205 changed files with 2,125 additions and 2,475 deletions.
7 changes: 7 additions & 0 deletions doc/release-notes/9782-juni5-transition.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Migrating all test to JUnit 5
With this release, we transition all of our test cases (see `src/test/`) to use JUnit 5 only.
Moving forward from JUnit 4 will allow writing tests in more concise and easier ways.
The tests themselves have not been altered, but updated to match JUnit 5 ways.
They have not been extended or dropped coverage; this is mostly a preparation of things to come in the future.
If you are writing tests in JUnit 4 in your feature branches, you need to migrate.
The development guides section of testing has been updated as well.
35 changes: 21 additions & 14 deletions doc/sphinx-guides/source/developers/testing.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@ Writing Unit Tests with JUnit
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

We are aware that there are newer testing tools such as TestNG, but we use `JUnit <http://junit.org>`_ because it's tried and true.
We support both (legacy) JUnit 4.x tests (forming the majority of our tests) and
newer JUnit 5 based testing.
We support JUnit 5 based testing and require new tests written with it.
(Since Dataverse 6.0, we migrated all of our tests formerly based on JUnit 4.)

NOTE: When adding new tests, you should give JUnit 5 a go instead of adding more dependencies to JUnit 4.x.

If writing tests is new to you, poke around existing unit tests which all end in ``Test.java`` and live under ``src/test``. Each test is annotated with ``@Test`` and should have at least one assertion which specifies the expected result. In Netbeans, you can run all the tests in it by clicking "Run" -> "Test File". From the test file, you should be able to navigate to the code that's being tested by right-clicking on the file and clicking "Navigate" -> "Go to Test/Tested class". Likewise, from the code, you should be able to use the same "Navigate" menu to go to the tests.
If writing tests is new to you, poke around existing unit tests which all end in ``Test.java`` and live under ``src/test``.
Each test is annotated with ``@Test`` and should have at least one assertion which specifies the expected result.
In Netbeans, you can run all the tests in it by clicking "Run" -> "Test File".
From the test file, you should be able to navigate to the code that's being tested by right-clicking on the file and clicking "Navigate" -> "Go to Test/Tested class".
Likewise, from the code, you should be able to use the same "Navigate" menu to go to the tests.

NOTE: Please remember when writing tests checking possibly localized outputs to check against ``en_US.UTF-8`` and ``UTC``
l10n strings!
Expand All @@ -62,22 +64,24 @@ Refactoring Code to Make It Unit-Testable

Existing code is not necessarily written in a way that lends itself to easy testing. Generally speaking, it is difficult to write unit tests for both JSF "backing" beans (which end in ``Page.java``) and "service" beans (which end in ``Service.java``) because they require the database to be running in order to test them. If service beans can be exercised via API they can be tested with integration tests (described below) but a good technique for making the logic testable it to move code to "util beans" (which end in ``Util.java``) that operate on Plain Old Java Objects (POJOs). ``PrivateUrlUtil.java`` is a good example of moving logic from ``PrivateUrlServiceBean.java`` to a "util" bean to make the code testable.

Parameterized Tests and JUnit Theories
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Parameterized Tests
^^^^^^^^^^^^^^^^^^^

Often times you will want to test a method multiple times with similar values.
In order to avoid test bloat (writing a test for every data combination),
JUnit offers Data-driven unit tests. This allows a test to be run for each set
of defined data values.

JUnit 4 uses ``Parameterized.class`` and ``Theories.class``. For reference, take a look at issue https://github.com/IQSS/dataverse/issues/5619.

JUnit 5 doesn't offer theories (see `jqwik <https://jqwik.net>`_ for this), but
greatly extended parameterized testing. Some guidance how to write those:
JUnit 5 offers great parameterized testing. Some guidance how to write those:

- https://junit.org/junit5/docs/current/user-guide/#writing-tests-parameterized-tests
- https://www.baeldung.com/parameterized-tests-junit-5
- https://blog.codefx.org/libraries/junit-5-parameterized-tests/
- See also some examples in our codebase.
- See also many examples in our codebase.

Note that JUnit 5 also offers support for custom test parameter resolvers. This enables keeping tests cleaner,
as preparation might happen within some extension and the test code is more focused on the actual testing.
See https://junit.org/junit5/docs/current/user-guide/#extensions-parameter-resolution for more information.

JUnit 5 Test Helper Extensions
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -116,11 +120,14 @@ In addition, there is a writeup on "The Testable Command" at https://github.com/
Running Non-Essential (Excluded) Unit Tests
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

You should be aware that some unit tests have been deemed "non-essential" and have been annotated with ``@Category(NonEssentialTests.class)`` and are excluded from the "dev" Maven profile, which is the default profile. All unit tests (that have not been annotated with ``@Ignore``), including these non-essential tests, are run from continuous integration systems such as Jenkins and GitHub Actions with the following ``mvn`` command that invokes a non-default profile:
You should be aware that some unit tests have been deemed "non-essential" and have been annotated with ``@Tag(Tags.NOT_ESSENTIAL_UNITTESTS)`` and are excluded from the "dev" Maven profile, which is the default profile.
All unit tests (that have not been annotated with ``@Disable``), including these non-essential tests, are run from continuous integration systems such as Jenkins and GitHub Actions with the following ``mvn`` command that invokes a non-default profile:

``mvn test -P all-unit-tests``

Generally speaking, unit tests have been flagged as non-essential because they are slow or because they require an Internet connection. You should not feel obligated to run these tests continuously but you can use the ``mvn`` command above to run them. To iterate on the unit test in Netbeans and execute it with "Run -> Test File", you must temporarily comment out the annotation flagging the test as non-essential.
Generally speaking, unit tests have been flagged as non-essential because they are slow or because they require an Internet connection.
You should not feel obligated to run these tests continuously but you can use the ``mvn`` command above to run them.
To iterate on the unit test in Netbeans and execute it with "Run -> Test File", you must temporarily comment out the annotation flagging the test as non-essential.

Integration Tests
-----------------
Expand Down
55 changes: 50 additions & 5 deletions modules/dataverse-parent/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,9 @@
<!-- Testing dependencies -->
<testcontainers.version>1.15.0</testcontainers.version>
<smallrye-mpconfig.version>2.10.1</smallrye-mpconfig.version>

<junit.version>4.13.1</junit.version>
<junit.jupiter.version>5.7.0</junit.jupiter.version>
<junit.vintage.version>${junit.jupiter.version}</junit.vintage.version>
<mockito.version>2.28.2</mockito.version>

<junit.jupiter.version>5.10.0</junit.jupiter.version>
<mockito.version>5.4.0</mockito.version>

<checkstyle.version>9.3</checkstyle.version>

Expand All @@ -193,6 +191,7 @@
<maven-source-plugin.version>3.2.1</maven-source-plugin.version>
<maven-javadoc-plugin.version>3.4.1</maven-javadoc-plugin.version>
<maven-flatten-plugin.version>1.3.0</maven-flatten-plugin.version>
<maven-enforcer-plugin.version>3.3.0</maven-enforcer-plugin.version>

<maven-checkstyle-plugin.version>3.1.2</maven-checkstyle-plugin.version>
<nexus-staging-plugin.version>1.6.13</nexus-staging-plugin.version>
Expand Down Expand Up @@ -255,6 +254,11 @@
<artifactId>maven-failsafe-plugin</artifactId>
<version>${maven-failsafe-plugin.version}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<version>${maven-enforcer-plugin.version}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
Expand Down Expand Up @@ -314,6 +318,47 @@
</plugin>
</plugins>
</pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<executions>
<execution>
<id>no-junit4</id>
<phase>generate-test-resources</phase>
<goals>
<goal>enforce</goal>
</goals>
<configuration>
<rules>
<bannedDependencies>
<excludes>
<exclude>junit:junit:*:*:test</exclude>
<exclude>org.junit:junit:*:*:test</exclude>
<exclude>org.junit.vintage:*:*:*:test</exclude>
</excludes>
</bannedDependencies>
</rules>
</configuration>
</execution>
<execution>
<id>general-reqs</id>
<goals>
<goal>enforce</goal>
</goals>
<phase>initialize</phase>
<configuration>
<rules>
<banDuplicatePomDependencyVersions/>
<requireJavaVersion>
<version>[${target.java.version}.0,)</version>
</requireJavaVersion>
</rules>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

<!--Maven checks for dependencies from these repos in the order shown in the pom.xml
Expand Down
26 changes: 13 additions & 13 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,12 @@
<artifactId>ezid</artifactId>
<version>1.0.0</version>
<type>jar</type>
<exclusions>
<exclusion>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.jsoup</groupId>
Expand Down Expand Up @@ -542,18 +548,6 @@
<version>${junit.jupiter.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>${junit.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
<version>${junit.vintage.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-library</artifactId>
Expand Down Expand Up @@ -594,6 +588,12 @@
<groupId>org.testcontainers</groupId>
<artifactId>testcontainers</artifactId>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
Expand Down Expand Up @@ -781,7 +781,7 @@
<activeByDefault>true</activeByDefault>
</activation>
<properties>
<testsToExclude>edu.harvard.iq.dataverse.NonEssentialTests</testsToExclude>
<testsToExclude>not-essential-unittests</testsToExclude>
</properties>
</profile>
<profile>
Expand Down
9 changes: 4 additions & 5 deletions src/test/java/edu/harvard/iq/dataverse/AuxiliaryFileServiceBeanTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@
import java.util.Arrays;
import java.util.List;
import jakarta.persistence.EntityManager;
import jakarta.persistence.Query;
import jakarta.persistence.TypedQuery;

import static org.junit.Assert.assertEquals;
import org.junit.Test;
import org.junit.Before;
import static org.junit.jupiter.api.Assertions.assertEquals;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentMatchers;

Expand All @@ -25,7 +24,7 @@ public class AuxiliaryFileServiceBeanTest {
List<String> types;
DataFile dataFile;

@Before
@BeforeEach
public void setup() {
svc = new AuxiliaryFileServiceBean();
svc.em = mock(EntityManager.class);
Expand Down
18 changes: 9 additions & 9 deletions src/test/java/edu/harvard/iq/dataverse/CartTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
package edu.harvard.iq.dataverse;

import static org.junit.Assert.fail;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import java.util.List;
import java.util.Map.Entry;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

public class CartTest {

private Cart cart;
private String title;
private String persistentId;

@Before
@BeforeEach
public void setUp() {
this.cart = new Cart();
this.title = "title";
this.persistentId = "persistentId";
}

@After
@AfterEach
public void tearDwon() {
this.cart = null;
this.title = null;
Expand Down
10 changes: 6 additions & 4 deletions src/test/java/edu/harvard/iq/dataverse/DataFileCategoryServiceBeanTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
import edu.harvard.iq.dataverse.util.BundleUtil;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
import org.mockito.junit.jupiter.MockitoExtension;

import java.util.Arrays;
import java.util.List;
Expand All @@ -19,7 +21,7 @@
*
* @author adaybujeda
*/
@RunWith(MockitoJUnitRunner.class)
@ExtendWith(MockitoExtension.class)
public class DataFileCategoryServiceBeanTest {

@Mock
Expand Down
9 changes: 5 additions & 4 deletions src/test/java/edu/harvard/iq/dataverse/DataFileServiceBeanTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package edu.harvard.iq.dataverse;

import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.*;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

/**
* Test that the DataFileServiceBean classifies DataFiles correctly.
Expand All @@ -27,7 +28,7 @@ public DataFileServiceBeanTest() {
private DataFileServiceBean dataFileServiceBean;


@Before
@BeforeEach
public void setUp() {
fileWoContentType = createDataFile(null);
fileWithBogusContentType = createDataFile("foo/bar");
Expand Down
54 changes: 17 additions & 37 deletions src/test/java/edu/harvard/iq/dataverse/DatasetAuthorTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,46 +1,26 @@
package edu.harvard.iq.dataverse;

import static org.junit.Assert.assertEquals;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

import java.util.Arrays;
import java.util.Collection;
import static org.junit.jupiter.api.Assertions.assertEquals;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

@RunWith(Parameterized.class)
public class DatasetAuthorTest {

public String idType;
public String idValue;
public String expectedIdentifierAsUrl;

public DatasetAuthorTest(String idType, String idValue, String expectedIdentifierAsUrl) {
this.idType = idType;
this.idValue = idValue;
this.expectedIdentifierAsUrl = expectedIdentifierAsUrl;
}

@Parameters
public static Collection<String[]> parameters() {
return Arrays.asList(new String[][] {
{ "ORCID", "0000-0002-1825-0097", "https://orcid.org/0000-0002-1825-0097" },
{ "ISNI", "0000000121032683", "http://www.isni.org/isni/0000000121032683"},
{ "LCNA", "n82058243", "http://id.loc.gov/authorities/names/n82058243" },
{ "VIAF", "172389567", "https://viaf.org/viaf/172389567" },
{ "GND", "4079154-3", "https://d-nb.info/gnd/4079154-3" },
{ "ResearcherID", "634082", "https://publons.com/researcher/634082/" },
{ "ResearcherID", "AAW-9289-2021", "https://publons.com/researcher/AAW-9289-2021/" },
{ "ResearcherID", "J-9733-2013", "https://publons.com/researcher/J-9733-2013/" },
{ "ScopusID", "6602344670", "https://www.scopus.com/authid/detail.uri?authorId=6602344670" },
{ null, null, null, },
});
}

@Test
public void getIdentifierAsUrl() {
@ParameterizedTest
@CsvSource(value = {
"ORCID,0000-0002-1825-0097,https://orcid.org/0000-0002-1825-0097",
"ISNI,0000000121032683,http://www.isni.org/isni/0000000121032683",
"LCNA,n82058243,http://id.loc.gov/authorities/names/n82058243",
"VIAF,172389567,https://viaf.org/viaf/172389567",
"GND,4079154-3,https://d-nb.info/gnd/4079154-3",
"ResearcherID,634082,https://publons.com/researcher/634082/",
"ResearcherID,AAW-9289-2021,https://publons.com/researcher/AAW-9289-2021/",
"ResearcherID,J-9733-2013,https://publons.com/researcher/J-9733-2013/",
"ScopusID,6602344670,https://www.scopus.com/authid/detail.uri?authorId=6602344670",
"NULL,NULL,NULL"
}, nullValues = "NULL")
void getIdentifierAsUrl(String idType, String idValue, String expectedIdentifierAsUrl) {
DatasetAuthor datasetAuthor = new DatasetAuthor();
if (idType !=null && idValue != null) {
datasetAuthor.setIdType(idType);
Expand Down
Loading

0 comments on commit 497c578

Please sign in to comment.