Migrate to native gradle test task (#3987)

* Migrate to native gradle test task

* Run correct test report task on build

* Fix build

* More detailed output about failed tests

* Fix tests

.travis.yml

@@ -50,7 +50,7 @@ script:
   - if [ "$TEST_SUITE" != "guiTest" ] && [ "$TEST_SUITE" != "checkstyle" ] && [ "$TEST_SUITE" != "codecov" ]; then ./gradlew $TEST_SUITE $OPTIONS --scan; fi
   - if [ "$TEST_SUITE" == "checkstyle" ]; then ./gradlew checkstyleMain checkstyleTest checkstyleJmh; fi
   - if [ "$TEST_SUITE" == "guiTest" ]; then ./buildres/gui-tests.sh; fi
-  - if [ "$TEST_SUITE" == "codecov" ]; then ./gradlew jacocoJunit5TestReport; bash <(curl -s https://codecov.io/bash); fi
+  - if [ "$TEST_SUITE" == "codecov" ]; then ./gradlew jacocoTestReport; bash <(curl -s https://codecov.io/bash); fi
   - if [ "$DEPENDENCY_UPDATES" == "check" ]; then ./gradlew -q checkOutdatedDependencies; fi
 
 after_failure:

build.gradle

@@ -12,9 +12,6 @@ buildscript {
             url 'https://oss.sonatype.org/content/groups/public'
         }
     }
-    dependencies {
-        classpath 'org.junit.platform:junit-platform-gradle-plugin:1.2.0-M1'
-    }
 }
 
 plugins {
@@ -41,7 +38,6 @@ apply plugin: 'jacoco'
 apply plugin: 'install4j'
 apply plugin: 'me.champeau.gradle.jmh'
 apply plugin: 'checkstyle'
-apply plugin: 'org.junit.platform.gradle.plugin'
 
 apply from: 'eclipse.gradle'
 apply from: 'localization.gradle'
@@ -154,10 +150,10 @@ dependencies {
     compile group: 'com.microsoft.azure', name: 'applicationinsights-core', version: '2.0.2'
     compile group: 'com.microsoft.azure', name: 'applicationinsights-logging-log4j2', version: '2.0.2'
 
-    testCompile 'org.junit.jupiter:junit-jupiter-api:5.2.0-RC1'
+    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.2.0-RC1'
     testCompile 'org.junit.jupiter:junit-jupiter-params:5.2.0-RC1'
-    testRuntime 'org.junit.jupiter:junit-jupiter-engine:5.2.0-RC1'
-    testRuntime 'org.junit.vintage:junit-vintage-engine:5.2.0-RC1'
+    testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.2.0-RC1'
+    testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.2.0-RC1'
     testCompile 'org.junit.platform:junit-platform-launcher:1.2.0-RC1'
     testRuntime 'org.apache.logging.log4j:log4j-core:2.11.0'
     testRuntime 'org.apache.logging.log4j:log4j-jul:2.11.0'
@@ -311,14 +307,16 @@ javadoc {
 }
 
 // Test tasks
-junitPlatform {
-    filters {
-        tags {
-            exclude 'DatabaseTest', 'FetcherTest', 'GUITest', 'org.jabref.testutils.category.FetcherTest', 'org.jabref.testutils.category.GUITest'
-        }
+test {
+    useJUnitPlatform {
+        excludeTags 'DatabaseTest', 'FetcherTest', 'GUITest', 'org.jabref.testutils.category.FetcherTest', 'org.jabref.testutils.category.GUITest'
+    }
+
+    testLogging {
+        // set options for log level LIFECYCLE
+        events "failed"
+        exceptionFormat "full"
     }
-
-    logManager 'org.apache.logging.log4j.jul.LogManager'
 }
 
 task databaseTest(type: Test) {
@@ -355,8 +353,8 @@ tasks.withType(Test) {
 }
 
 task jacocoMerge(type: JacocoMerge) {
-    executionData file("$buildDir/jacoco/junitPlatformTest.exec"), file("$buildDir/jacoco/databaseTest.exec"), file("$buildDir/jacoco/fetcherTest.exec")
-    dependsOn junitPlatformTest, databaseTest, fetcherTest
+    executionData file("$buildDir/jacoco/test.exec"), file("$buildDir/jacoco/databaseTest.exec"), file("$buildDir/jacoco/fetcherTest.exec")
+    dependsOn test, databaseTest, fetcherTest
 }
 
 jacocoTestReport {
@@ -369,27 +367,6 @@ jacocoTestReport {
     }
 }
 
-afterEvaluate {
-    def junitPlatformTest = tasks.junitPlatformTest
-
-    jacoco {
-        applyTo(junitPlatformTest)
-    }
-
-    task jacocoJunit5TestReport(type: JacocoReport) {
-        executionData jacocoMerge.destinationFile
-        dependsOn jacocoMerge
-        sourceSets sourceSets.main
-        sourceDirectories = files(sourceSets.main.allSource.srcDirs)
-        classDirectories = files(sourceSets.main.output)
-
-        reports {
-            xml.enabled true
-            html.enabled true
-        }
-    }
-}
-
 // Code quality tasks
 checkstyle {
     // do not use other packages for checkstyle, excluding gen(erated) sources

src/main/java/org/jabref/logic/importer/fetcher/IacrEprintFetcher.java

@@ -25,9 +25,9 @@
 import org.jabref.logic.net.URLDownload;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.FieldName;
+import org.jabref.model.strings.StringUtil;
 import org.jabref.model.util.DummyFileUpdateMonitor;
 
-import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -179,7 +179,7 @@ private String getHtml(String url) throws FetcherException {
     }
 
     private String getRequiredValueBetween(String from, String to, String haystack) throws FetcherException {
-        String value = StringUtils.substringBetween(haystack, from, to);
+        String value = StringUtil.substringBetween(haystack, from, to);
         if (value == null) {
             throw new FetcherException(Localization.lang("Entry from %0 could not be parsed.", "IACR"));
         } else {

src/main/java/org/jabref/logic/importer/fileformat/RisImporter.java

@@ -49,8 +49,7 @@ public ParserResult importDatabase(BufferedReader reader) throws IOException {
         List<BibEntry> bibitems = new ArrayList<>();
 
         //use optional here, so that no exception will be thrown if the file is empty
-        Optional<String> OptionalLines = reader.lines().reduce((line, nextline) -> line + "\n" + nextline);
-        String linesAsString = OptionalLines.isPresent() ? OptionalLines.get() : "";
+        String linesAsString = reader.lines().reduce((line, nextline) -> line + "\n" + nextline).orElse("");
 
         String[] entries = linesAsString.replace("\u2013", "-").replace("\u2014", "--").replace("\u2015", "--")
                 .split("ER  -.*\\n");
@@ -207,7 +206,7 @@ public ParserResult importDatabase(BufferedReader reader) throws IOException {
                         }
                     } else if ("U1".equals(tag) || "U2".equals(tag) || "N1".equals(tag)) {
                         if (!comment.isEmpty()) {
-                            comment = comment + " ";
+                            comment = comment + OS.NEWLINE;
                         }
                         comment = comment + value;
                     }  else if ("M3".equals(tag) || "DO".equals(tag)) {
@@ -269,7 +268,7 @@ else if ("AV".equals(tag)) {
             BibEntry entry = new BibEntry(type);
             entry.setField(fields);
             // month has a special treatment as we use the separate method "setMonth" of BibEntry instead of directly setting the value
-            month.ifPresent(parsedMonth -> entry.setMonth(parsedMonth));
+            month.ifPresent(entry::setMonth);
             bibitems.add(entry);
 
         }

src/main/java/org/jabref/model/entry/Month.java

@@ -8,8 +8,6 @@
 
 import org.jabref.model.strings.StringUtil;
 
-import org.apache.commons.lang3.StringUtils;
-
 /**
  * Represents a Month of the Year.
  */
@@ -124,8 +122,8 @@ private static Optional<Month> parseGermanShortMonth(String value) {
         }
 
         try {
-            YearMonth yearMonth = YearMonth.parse("1969-" + StringUtils.capitalize(value),
-            		DateTimeFormatter.ofPattern("yyyy-MMM", Locale.GERMAN));
+            YearMonth yearMonth = YearMonth.parse("1969-" + StringUtil.capitalizeFirst(value),
+                    DateTimeFormatter.ofPattern("yyyy-MMM", Locale.GERMAN));
             return Month.getMonthByNumber(yearMonth.getMonthValue());
         } catch (DateTimeParseException e) {
             return Optional.empty();

src/main/java/org/jabref/model/strings/StringUtil.java

@@ -691,8 +691,7 @@ public static String stripAccents(String searchQuery) {
     }
 
     /**
-     * Make first character of String uppercase, and the
-     * rest lowercase.
+     * Make first character of String uppercase, and the rest lowercase.
      */
     public static String capitalizeFirst(String toCapitalize) {
         if (toCapitalize.length() > 1) {
@@ -719,4 +718,8 @@ public static List<String> getStringAsWords(String text) {
     public static boolean containsIgnoreCase(String text, String searchString) {
         return StringUtils.containsIgnoreCase(text, searchString);
     }
+
+    public static String substringBetween(String str, String open, String close) {
+        return StringUtils.substringBetween(str, open, close);
+    }
 }

src/test/java/org/jabref/architecture/MainArchitectureTests.java

@@ -39,7 +39,7 @@ public class MainArchitectureTests {
     private static Map<String, List<String>> exceptions;
 
     @BeforeAll
-    public static void setUp() {
+    static void setUp() {
         exceptions = new HashMap<>();
         // Add exceptions for the architectural test here
         // Note that bending the architectural constraints should not be done inconsiderately
@@ -61,7 +61,7 @@ public static void setUp() {
         exceptions.put(PACKAGE_ORG_JABREF_MODEL, modelExceptions);
     }
 
-    public static Stream<Arguments> getPackages() {
+    private static Stream<Arguments> getPackages() {
 
         return Stream.of(
                 Arguments.of(PACKAGE_ORG_JABREF_LOGIC, PACKAGE_JAVA_AWT),
@@ -80,7 +80,7 @@ public static Stream<Arguments> getPackages() {
 
     @ParameterizedTest(name = "{index} -- is {0} independent of {1}?")
     @MethodSource("getPackages")
-    public void firstPackageIsIndependentOfSecondPackage(String firstPackage, String secondPackage) throws IOException {
+    void firstPackageIsIndependentOfSecondPackage(String firstPackage, String secondPackage) throws IOException {
         Predicate<String> isExceptionPackage = (s) -> (s.startsWith("import " + secondPackage)
                 || s.startsWith("import static " + secondPackage))
                 && exceptions.getOrDefault(firstPackage, Collections.emptyList())

src/test/java/org/jabref/logic/importer/ImportFormatReaderTestParameterless.java

@@ -16,40 +16,40 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-public class ImportFormatReaderTestParameterless {
+class ImportFormatReaderTestParameterless {
 
     private ImportFormatReader reader;
     private final FileUpdateMonitor fileMonitor = new DummyFileUpdateMonitor();
 
     @BeforeEach
-    public void setUp() {
+    void setUp() {
         reader = new ImportFormatReader();
         ImportFormatPreferences importFormatPreferences = mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS);
         when(importFormatPreferences.getEncoding()).thenReturn(StandardCharsets.UTF_8);
         reader.resetImportFormats(importFormatPreferences, mock(XmpPreferences.class), fileMonitor);
     }
 
     @Test
-    public void importUnknownFormatThrowsExceptionIfNoMatchingImporterWasFound() throws Exception {
+    void importUnknownFormatThrowsExceptionIfNoMatchingImporterWasFound() throws Exception {
         Path file = Paths.get(ImportFormatReaderTestParameterless.class.getResource("fileformat/emptyFile.xml").toURI());
-        assertThrows(NullPointerException.class, () -> reader.importUnknownFormat(file, fileMonitor));
+        assertThrows(ImportException.class, () -> reader.importUnknownFormat(file, fileMonitor));
     }
 
     @Test
-    public void testNullImportUnknownFormatPath() throws Exception {
+    void importUnknownFormatThrowsExceptionIfPathIsNull() throws Exception {
         assertThrows(NullPointerException.class, () -> reader.importUnknownFormat(null, fileMonitor));
 
     }
 
     @Test
-    public void testNullImportUnknownFormatString() throws Exception {
+    void importUnknownFormatThrowsExceptionIfDataIsNull() throws Exception {
         assertThrows(NullPointerException.class, () -> reader.importUnknownFormat(null));
 
     }
 
     @Test
-    public void importFromFileWithUnknownFormatThrowsException() throws Exception {
-        assertThrows(NullPointerException.class, () -> reader.importFromFile("someunknownformat", Paths.get("somepath")));
+    void importFromFileWithUnknownFormatThrowsException() throws Exception {
+        assertThrows(ImportException.class, () -> reader.importFromFile("someunknownformat", Paths.get("somepath")));
     }
 
 }

src/test/java/org/jabref/logic/importer/fileformat/ModsImporterTestFiles.java

@@ -6,32 +6,39 @@
 
 import org.jabref.logic.importer.ImportFormatPreferences;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.mockito.Answers;
 
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
-public class ModsImporterTestFiles {
+class ModsImporterTestFiles {
 
     private static final String FILE_ENDING = ".xml";
+    private ImportFormatPreferences importFormatPreferences;
 
     private static Stream<String> fileNames() throws IOException {
-        Predicate<String> fileName = name -> name.startsWith("MODS")
-                && name.endsWith(FILE_ENDING);
+        Predicate<String> fileName = name -> name.startsWith("MODS") && name.endsWith(FILE_ENDING);
         return ImporterTestEngine.getTestFiles(fileName).stream();
     }
 
+    @BeforeEach
+    void setUp() {
+        importFormatPreferences = mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS);
+        when(importFormatPreferences.getKeywordSeparator()).thenReturn(',');
+    }
+
     @ParameterizedTest
     @MethodSource("fileNames")
-    public void testIsRecognizedFormat(String fileName) throws IOException {
-        ImporterTestEngine.testIsRecognizedFormat(new ModsImporter(mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS)), fileName);
+    void testIsRecognizedFormat(String fileName) throws IOException {
+        ImporterTestEngine.testIsRecognizedFormat(new ModsImporter(importFormatPreferences), fileName);
     }
 
     @ParameterizedTest
     @MethodSource("fileNames")
-    public void testImportEntries(String fileName) throws Exception {
-        ImporterTestEngine.testImportEntries(new ModsImporter(mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS)
-        ), fileName, FILE_ENDING);
+    void testImportEntries(String fileName) throws Exception {
+        ImporterTestEngine.testImportEntries(new ModsImporter(importFormatPreferences), fileName, FILE_ENDING);
     }
 }

src/test/java/org/jabref/logic/importer/fileformat/RISImporterTestFiles.java

@@ -7,7 +7,7 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
-public class RISImporterTestFiles {
+class RISImporterTestFiles {
 
     private static final String FILE_ENDING = ".ris";
 
@@ -18,13 +18,13 @@ private static Stream<String> fileNames() throws IOException {
 
     @ParameterizedTest
     @MethodSource("fileNames")
-    public void testIsRecognizedFormat(String fileName) throws IOException {
+    void testIsRecognizedFormat(String fileName) throws IOException {
         ImporterTestEngine.testIsRecognizedFormat(new RisImporter(), fileName);
     }
 
     @ParameterizedTest
     @MethodSource("fileNames")
-    public void testImportEntries(String fileName) throws Exception {
+    void testImportEntries(String fileName) throws Exception {
         ImporterTestEngine.testImportEntries(new RisImporter(), fileName, FILE_ENDING);
     }
 }