From 95694c8a0fb216573b6abf9c054af37e15211734 Mon Sep 17 00:00:00 2001 From: BShaq Date: Sat, 1 May 2021 23:42:58 +0200 Subject: [PATCH 01/10] transformed to parameterized tests Transformed StringLengthComparatorTest & MinifyNameListFormatterTest to parameterized tests, since they've got multiple assertions within one unit test included (Test Smell: Assertion Roulette) --- .../minifier/MinifyNameListFormatterTest.java | 33 ++++++----- .../strings/StringLengthComparatorTest.java | 57 +++++++++++-------- 2 files changed, 50 insertions(+), 40 deletions(-) diff --git a/src/test/java/org/jabref/logic/formatter/minifier/MinifyNameListFormatterTest.java b/src/test/java/org/jabref/logic/formatter/minifier/MinifyNameListFormatterTest.java index 28d6140e248..eccff6f9f15 100644 --- a/src/test/java/org/jabref/logic/formatter/minifier/MinifyNameListFormatterTest.java +++ b/src/test/java/org/jabref/logic/formatter/minifier/MinifyNameListFormatterTest.java @@ -1,7 +1,11 @@ package org.jabref.logic.formatter.minifier; +import java.util.stream.Stream; + import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -17,21 +21,20 @@ public void setUp() { formatter = new MinifyNameListFormatter(); } - @Test - public void minifyAuthorNames() { - expectCorrect("Simon Harrer", "Simon Harrer"); - expectCorrect("Simon Harrer and others", "Simon Harrer and others"); - expectCorrect("Simon Harrer and Jörg Lenhard", "Simon Harrer and Jörg Lenhard"); - expectCorrect("Simon Harrer and Jörg Lenhard and Guido Wirtz", "Simon Harrer and others"); - expectCorrect("Simon Harrer and Jörg Lenhard and Guido Wirtz and others", "Simon Harrer and others"); - } - - @Test - public void formatExample() { - expectCorrect(formatter.getExampleInput(), "Stefan Kolb and others"); + @ParameterizedTest + @MethodSource("provideAuthorNames") + void minifyAuthorNames(String expectedAuthorNames, String originalAuthorNames) { + assertEquals(expectedAuthorNames, formatter.format(originalAuthorNames)); } - private void expectCorrect(String input, String expected) { - assertEquals(expected, formatter.format(input)); + private static Stream provideAuthorNames() { + return Stream.of( + Arguments.of("Simon Harrer", "Simon Harrer"), + Arguments.of("Simon Harrer and others", "Simon Harrer and others"), + Arguments.of("Simon Harrer and Jörg Lenhard", "Simon Harrer and Jörg Lenhard"), + Arguments.of("Simon Harrer and others", "Simon Harrer and Jörg Lenhard and Guido Wirtz"), + Arguments.of("Simon Harrer and others", "Simon Harrer and Jörg Lenhard and Guido Wirtz and others"), + Arguments.of("Stefan Kolb and others", new MinifyNameListFormatter().getExampleInput()) + ); } } diff --git a/src/test/java/org/jabref/logic/util/strings/StringLengthComparatorTest.java b/src/test/java/org/jabref/logic/util/strings/StringLengthComparatorTest.java index c2259f49427..deb965393fb 100644 --- a/src/test/java/org/jabref/logic/util/strings/StringLengthComparatorTest.java +++ b/src/test/java/org/jabref/logic/util/strings/StringLengthComparatorTest.java @@ -1,7 +1,14 @@ package org.jabref.logic.util.strings; +import java.util.stream.Stream; + +import org.jabref.logic.formatter.bibtexfields.NormalizePagesFormatter; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -14,32 +21,32 @@ public void setUp() { slc = new StringLengthComparator(); } - @Test - public void test() { - assertEquals(-1, slc.compare("AAA", "AA")); - assertEquals(0, slc.compare("AA", "AA")); - assertEquals(1, slc.compare("AA", "AAA")); - } - - @Test - public void emptyStringTest() { - assertEquals(-1, slc.compare("A", "")); - assertEquals(0, slc.compare("", "")); - assertEquals(1, slc.compare("", "A")); - } - - @Test - public void backslashTest() { - assertEquals(-1, slc.compare("\\\\", "A")); - assertEquals(0, slc.compare("\\", "A")); - assertEquals(0, slc.compare("\\", "\\")); - assertEquals(0, slc.compare("A", "\\")); - assertEquals(1, slc.compare("A", "\\\\")); + @ParameterizedTest + @MethodSource("tests") + void compareStringLength(int comparisonResult, String firstString, String secondString) { + assertEquals(comparisonResult, slc.compare(firstString, secondString)); } - @Test - public void emptyStringAndBackslashTest() { - assertEquals(-1, slc.compare("\\", "")); - assertEquals(1, slc.compare("", "\\")); + private static Stream tests() { + return Stream.of( + Arguments.of(-1, "AAA", "AA"), + Arguments.of(0, "AA", "AA"), + Arguments.of(1, "AA", "AAA"), + + // empty strings + Arguments.of(-1, "A", ""), + Arguments.of(0, "", ""), + Arguments.of(1, "", "A"), + + // backslash + Arguments.of(-1, "\\\\", "A"), + Arguments.of(0, "\\", "A"), + Arguments.of(0, "\\", "\\"), + Arguments.of(0, "A", "\\"), + Arguments.of(1, "A", "\\\\"), + + // empty string + backslash + Arguments.of(-1, "\\", ""), + Arguments.of(1, "", "\\")); } } From 2e154d434c0fff2fc33b88a89e165f6c7f8e94f9 Mon Sep 17 00:00:00 2001 From: BShaq Date: Sun, 2 May 2021 14:03:41 +0200 Subject: [PATCH 02/10] refactor to parameterized tests There exists unit tests in these test classes that have multiple assertions (Test smell: Assertion Roulette). I therefore have refactored those unit tests, either by incorporating them into existing parameterized tests or by creating new parameterized tests. --- .../casechanger/CapitalizeFormatterTest.java | 13 ++--- .../logic/layout/format/DOICheckTest.java | 55 +++++++++++-------- .../format/RemoveBracketsAddCommaTest.java | 24 ++++++-- .../logic/layout/format/RemoveTildeTest.java | 33 +++++++---- 4 files changed, 75 insertions(+), 50 deletions(-) diff --git a/src/test/java/org/jabref/logic/formatter/casechanger/CapitalizeFormatterTest.java b/src/test/java/org/jabref/logic/formatter/casechanger/CapitalizeFormatterTest.java index 5cba9c90172..6210e48b96b 100644 --- a/src/test/java/org/jabref/logic/formatter/casechanger/CapitalizeFormatterTest.java +++ b/src/test/java/org/jabref/logic/formatter/casechanger/CapitalizeFormatterTest.java @@ -19,20 +19,13 @@ public void setUp() { formatter = new CapitalizeFormatter(); } - @Test - public void test() { - assertEquals("Upper Each First", formatter.format("upper each First")); - assertEquals("Upper Each First {NOT} {this}", formatter.format("upper each first {NOT} {this}")); - assertEquals("Upper Each First {N}ot {t}his", formatter.format("upper each first {N}OT {t}his")); - } - @Test public void formatExample() { assertEquals("I Have {a} Dream", formatter.format(formatter.getExampleInput())); } @ParameterizedTest(name = "input={0}, formattedStr={1}") - @CsvSource({ + @CsvSource(value = { "{}, {}", // {} "{upper, {upper", // unmatched braces "upper, Upper", // single word lower case @@ -41,13 +34,15 @@ public void formatExample() { "upper each first, Upper Each First", // multiple words lower case "Upper Each First, Upper Each First", // multiple words correct "UPPER EACH FIRST, Upper Each First", // multiple words upper case + "upper each First, Upper Each First", // multiple words in lower and upper case "{u}pp{e}r, {u}pp{e}r", // single word lower case with {} "{U}pp{e}r, {U}pp{e}r", // single word correct with {} "{U}PP{E}R, {U}pp{E}r", // single word upper case with {} "upper each {NOT} first, Upper Each {NOT} First", // multiple words lower case with {} "Upper {E}ach {NOT} First, Upper {E}ach {NOT} First", // multiple words correct with {} "UPPER {E}ACH {NOT} FIRST, Upper {E}ach {NOT} First", // multiple words upper case with {} - + "upper each first {NOT} {this}, Upper Each First {NOT} {this}", // multiple words in lower and upper case with {} + "upper each first {N}OT {t}his, Upper Each First {N}ot {t}his", // multiple words in lower and upper case with {} part 2 }) public void testInputs(String input, String expectedResult) { String formattedStr = formatter.format(input); diff --git a/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java b/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java index e14af93ecbd..70001fbd2dc 100644 --- a/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java +++ b/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java @@ -1,38 +1,45 @@ package org.jabref.logic.layout.format; +import java.util.stream.Stream; + import org.jabref.logic.layout.LayoutFormatter; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.BeforeEach; import static org.junit.jupiter.api.Assertions.assertEquals; -public class DOICheckTest { - - @Test - public void testFormat() { - LayoutFormatter lf = new DOICheck(); - - assertEquals("", lf.format("")); - assertEquals(null, lf.format(null)); +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; - assertEquals("https://doi.org/10.1000/ISBN1-900512-44-0", lf.format("10.1000/ISBN1-900512-44-0")); - assertEquals("https://doi.org/10.1000/ISBN1-900512-44-0", - lf.format("http://dx.doi.org/10.1000/ISBN1-900512-44-0")); - - assertEquals("https://doi.org/10.1000/ISBN1-900512-44-0", - lf.format("http://doi.acm.org/10.1000/ISBN1-900512-44-0")); +public class DOICheckTest { - assertEquals("https://doi.org/10.1145/354401.354407", - lf.format("http://doi.acm.org/10.1145/354401.354407")); - assertEquals("https://doi.org/10.1145/354401.354407", lf.format("10.1145/354401.354407")); + private LayoutFormatter layoutFormatter; - // Works even when having a / at the front - assertEquals("https://doi.org/10.1145/354401.354407", lf.format("/10.1145/354401.354407")); + @BeforeEach + void setup() { + layoutFormatter = new DOICheck(); + } - // Obviously a wrong doi, will not change anything. - assertEquals("10", lf.format("10")); + @ParameterizedTest + @MethodSource("provideDOI") + void formatDOI(String formattedDOI, String originalDOI) { + assertEquals(formattedDOI, layoutFormatter.format(originalDOI)); + } - // Obviously a wrong doi, will not change anything. - assertEquals("1", lf.format("1")); + private static Stream provideDOI() { + return Stream.of( + Arguments.of("", ""), + Arguments.of(null, null), + Arguments.of("https://doi.org/10.1000/ISBN1-900512-44-0", "10.1000/ISBN1-900512-44-0"), + Arguments.of("https://doi.org/10.1000/ISBN1-900512-44-0", "http://dx.doi.org/10.1000/ISBN1-900512-44-0"), + Arguments.of("https://doi.org/10.1000/ISBN1-900512-44-0", "http://doi.acm.org/10.1000/ISBN1-900512-44-0"), + Arguments.of("https://doi.org/10.1145/354401.354407", "http://doi.acm.org/10.1145/354401.354407"), + Arguments.of("https://doi.org/10.1145/354401.354407", "10.1145/354401.354407"), + Arguments.of("https://doi.org/10.1145/354401.354407", "/10.1145/354401.354407"), + Arguments.of("10", "10"), + Arguments.of("1", "1") + + ); } } diff --git a/src/test/java/org/jabref/logic/layout/format/RemoveBracketsAddCommaTest.java b/src/test/java/org/jabref/logic/layout/format/RemoveBracketsAddCommaTest.java index e805bbf5490..c760b666ae8 100644 --- a/src/test/java/org/jabref/logic/layout/format/RemoveBracketsAddCommaTest.java +++ b/src/test/java/org/jabref/logic/layout/format/RemoveBracketsAddCommaTest.java @@ -1,12 +1,17 @@ package org.jabref.logic.layout.format; +import java.util.stream.Stream; + import org.jabref.logic.layout.LayoutFormatter; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + public class RemoveBracketsAddCommaTest { private LayoutFormatter formatter; @@ -15,10 +20,17 @@ public void setUp() { formatter = new RemoveBracketsAddComma(); } - @Test - public void testFormat() throws Exception { - assertEquals("some text,", formatter.format("{some text}")); - assertEquals("some text", formatter.format("{some text")); - assertEquals("some text,", formatter.format("some text}")); + @ParameterizedTest + @MethodSource("provideExamples") + void formatTextWithBrackets(String formattedString, String originalString) { + assertEquals(formattedString, formatter.format(originalString)); + } + + private static Stream provideExamples() { + return Stream.of( + Arguments.of("some text,", "{some text}"), + Arguments.of("some text", "{some text"), + Arguments.of("some text,", "some text}") + ); } } diff --git a/src/test/java/org/jabref/logic/layout/format/RemoveTildeTest.java b/src/test/java/org/jabref/logic/layout/format/RemoveTildeTest.java index 1961b3dfac5..17635c2b723 100644 --- a/src/test/java/org/jabref/logic/layout/format/RemoveTildeTest.java +++ b/src/test/java/org/jabref/logic/layout/format/RemoveTildeTest.java @@ -1,9 +1,13 @@ package org.jabref.logic.layout.format; +import java.util.stream.Stream; + import org.jabref.logic.layout.LayoutFormatter; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -15,15 +19,22 @@ public void setUp() { formatter = new RemoveTilde(); } - @Test - public void testFormatString() { - assertEquals("", formatter.format("")); - assertEquals("simple", formatter.format("simple")); - assertEquals(" ", formatter.format("~")); - assertEquals(" ", formatter.format("~~~")); - assertEquals(" \\~ ", formatter.format("~\\~~")); - assertEquals("\\\\ ", formatter.format("\\\\~")); - assertEquals("Doe Joe and Jane, M. and Kamp, J. A.", formatter.format("Doe Joe and Jane, M. and Kamp, J.~A.")); - assertEquals("T\\~olkien, J. R. R.", formatter.format("T\\~olkien, J.~R.~R.")); + @ParameterizedTest + @MethodSource("provideArguments") + void formatText(String formattedString, String originalString) { + assertEquals(formattedString, formatter.format(originalString)); + } + + private static Stream provideArguments() { + return Stream.of( + Arguments.of("", ""), + Arguments.of("simple", "simple"), + Arguments.of(" ", "~"), + Arguments.of(" ", "~~~"), + Arguments.of(" \\~ ", "~\\~~"), + Arguments.of("\\\\ ", "\\\\~"), + Arguments.of("Doe Joe and Jane, M. and Kamp, J. A.", "Doe Joe and Jane, M. and Kamp, J.~A."), + Arguments.of("T\\~olkien, J. R. R.", "T\\~olkien, J.~R.~R.") + ); } } From 467be292a553ecc3298f5da2ee9a138e61b30963 Mon Sep 17 00:00:00 2001 From: BShaq Date: Sun, 2 May 2021 14:13:24 +0200 Subject: [PATCH 03/10] moved object creation into setup to reduce test code duplication --- .../logic/layout/format/AuthorsTest.java | 119 ++++++++---------- .../logic/layout/format/FileLinkTest.java | 41 +++--- .../logic/layout/format/FirstPageTest.java | 33 ++--- 3 files changed, 88 insertions(+), 105 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java b/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java index 09c5f7fd2d3..64f80c8c080 100644 --- a/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java +++ b/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java @@ -2,6 +2,7 @@ import org.jabref.logic.layout.ParamLayoutFormatter; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; @@ -10,151 +11,139 @@ public class AuthorsTest { + private ParamLayoutFormatter authorsLayoutFormatter; + + @BeforeEach + void setup() { + authorsLayoutFormatter = new Authors(); + } + @Test public void testStandardUsage() { - ParamLayoutFormatter a = new Authors(); assertEquals("B. C. Bruce, C. Manson and J. Jumper", - a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper")); + authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper")); } @Test public void testStandardUsageOne() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("fullname, LastFirst, Comma, Comma"); - assertEquals("Bruce, Bob Croydon, Jumper, Jolly", a.format("Bob Croydon Bruce and Jolly Jumper")); + authorsLayoutFormatter.setArgument("fullname, LastFirst, Comma, Comma"); + assertEquals("Bruce, Bob Croydon, Jumper, Jolly", authorsLayoutFormatter.format("Bob Croydon Bruce and Jolly Jumper")); } @Test public void testStandardUsageTwo() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("initials"); - assertEquals("B. C. Bruce and J. Jumper", a.format("Bob Croydon Bruce and Jolly Jumper")); + authorsLayoutFormatter.setArgument("initials"); + assertEquals("B. C. Bruce and J. Jumper", authorsLayoutFormatter.format("Bob Croydon Bruce and Jolly Jumper")); } @Test public void testStandardUsageThree() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("fullname, LastFirst, Comma"); + authorsLayoutFormatter.setArgument("fullname, LastFirst, Comma"); assertEquals("Bruce, Bob Croydon, Manson, Charles and Jumper, Jolly", - a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper")); + authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper")); } @Test public void testStandardUsageFour() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("fullname, LastFirst, Comma, 2"); + authorsLayoutFormatter.setArgument("fullname, LastFirst, Comma, 2"); assertEquals("Bruce, Bob Croydon et al.", - a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper")); + authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper")); } @Test public void testStandardUsageFive() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("fullname, LastFirst, Comma, 3"); + authorsLayoutFormatter.setArgument("fullname, LastFirst, Comma, 3"); assertEquals("Bruce, Bob Croydon et al.", - a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles")); + authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles")); } @Test public void testStandardUsageSix() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("fullname, LastFirst, Comma, 3, 2"); + authorsLayoutFormatter.setArgument("fullname, LastFirst, Comma, 3, 2"); assertEquals("Bruce, Bob Croydon, Manson, Charles et al.", - a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles")); + authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles")); } @Test public void testSpecialEtAl() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("fullname, LastFirst, Comma, 3, etal= and a few more"); + authorsLayoutFormatter.setArgument("fullname, LastFirst, Comma, 3, etal= and a few more"); assertEquals("Bruce, Bob Croydon and a few more", - a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles")); + authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles")); } @Test public void testStandardUsageNull() { - ParamLayoutFormatter a = new Authors(); - assertEquals("", a.format(null)); + assertEquals("", authorsLayoutFormatter.format(null)); } @Test public void testStandardOxford() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("Oxford"); + authorsLayoutFormatter.setArgument("Oxford"); assertEquals("B. C. Bruce, C. Manson, and J. Jumper", - a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper")); + authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper")); } @Test public void testStandardOxfordFullName() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("FullName,Oxford"); + authorsLayoutFormatter.setArgument("FullName,Oxford"); assertEquals("Bob Croydon Bruce, Charles Manson, and Jolly Jumper", - a.format("Bruce, Bob Croydon and Charles Manson and Jolly Jumper")); + authorsLayoutFormatter.format("Bruce, Bob Croydon and Charles Manson and Jolly Jumper")); } @Test public void testStandardCommaFullName() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("FullName,Comma,Comma"); + authorsLayoutFormatter.setArgument("FullName,Comma,Comma"); assertEquals("Bob Croydon Bruce, Charles Manson, Jolly Jumper", - a.format("Bruce, Bob Croydon and Charles Manson and Jolly Jumper")); + authorsLayoutFormatter.format("Bruce, Bob Croydon and Charles Manson and Jolly Jumper")); } @Test public void testStandardAmpFullName() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("FullName,Amp"); + authorsLayoutFormatter.setArgument("FullName,Amp"); assertEquals("Bob Croydon Bruce, Charles Manson & Jolly Jumper", - a.format("Bruce, Bob Croydon and Charles Manson and Jolly Jumper")); + authorsLayoutFormatter.format("Bruce, Bob Croydon and Charles Manson and Jolly Jumper")); } @Test public void testLastName() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("LastName"); + authorsLayoutFormatter.setArgument("LastName"); assertEquals("Bruce, von Manson and Jumper", - a.format("Bruce, Bob Croydon and Charles von Manson and Jolly Jumper")); + authorsLayoutFormatter.format("Bruce, Bob Croydon and Charles von Manson and Jolly Jumper")); } @Test public void testMiddleInitial() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("MiddleInitial"); + authorsLayoutFormatter.setArgument("MiddleInitial"); assertEquals("Bob C. Bruce, Charles K. von Manson and Jolly Jumper", - a.format("Bruce, Bob Croydon and Charles Kermit von Manson and Jumper, Jolly")); + authorsLayoutFormatter.format("Bruce, Bob Croydon and Charles Kermit von Manson and Jumper, Jolly")); } @Test public void testNoPeriod() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("NoPeriod"); + authorsLayoutFormatter.setArgument("NoPeriod"); assertEquals("B C Bruce, C K von Manson and J Jumper", - a.format("Bruce, Bob Croydon and Charles Kermit von Manson and Jumper, Jolly")); + authorsLayoutFormatter.format("Bruce, Bob Croydon and Charles Kermit von Manson and Jumper, Jolly")); } @Test public void testEtAl() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("2,1"); + authorsLayoutFormatter.setArgument("2,1"); assertEquals("B. C. Bruce et al.", - a.format("Bruce, Bob Croydon and Charles Kermit von Manson and Jumper, Jolly")); + authorsLayoutFormatter.format("Bruce, Bob Croydon and Charles Kermit von Manson and Jumper, Jolly")); } @Test public void testEtAlNotEnoughAuthors() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("2,1"); + authorsLayoutFormatter.setArgument("2,1"); assertEquals("B. C. Bruce and C. K. von Manson", - a.format("Bruce, Bob Croydon and Charles Kermit von Manson")); + authorsLayoutFormatter.format("Bruce, Bob Croydon and Charles Kermit von Manson")); } @Test public void testEmptyEtAl() { - ParamLayoutFormatter a = new Authors(); - a.setArgument("fullname, LastFirst, Comma, 3, etal="); + authorsLayoutFormatter.setArgument("fullname, LastFirst, Comma, 3, etal="); assertEquals("Bruce, Bob Croydon", - a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles")); + authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles")); } @ParameterizedTest(name = "arg={0}, formattedStr={1}") @@ -164,9 +153,8 @@ public void testEmptyEtAl() { "LastFirstFirstFirst, 'Bruce, B. C., C. Manson, J. Jumper and C. Chuckles'" // LastFirstFirstFirst }) public void testAuthorOrder(String arg, String expectedResult) { - ParamLayoutFormatter a = new Authors(); - a.setArgument(arg); - String formattedStr = a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles"); + authorsLayoutFormatter.setArgument(arg); + String formattedStr = authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles"); assertEquals(expectedResult, formattedStr); } @@ -180,9 +168,8 @@ public void testAuthorOrder(String arg, String expectedResult) { "InitialsNoSpace, 'B.C. Bruce, C. Manson, J. Jumper and C. Chuckles'" // InitialsNoSpace }) public void testAuthorABRV(String arg, String expectedResult) { - ParamLayoutFormatter a = new Authors(); - a.setArgument(arg); - String formattedStr = a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles"); + authorsLayoutFormatter.setArgument(arg); + String formattedStr = authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles"); assertEquals(expectedResult, formattedStr); } @@ -194,9 +181,8 @@ public void testAuthorABRV(String arg, String expectedResult) { "NoPeriod, 'B C Bruce, C Manson, J Jumper and C Chuckles'" // NoPeriod }) public void testAuthorPUNC(String arg, String expectedResult) { - ParamLayoutFormatter a = new Authors(); - a.setArgument(arg); - String formattedStr = a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles"); + authorsLayoutFormatter.setArgument(arg); + String formattedStr = authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles"); assertEquals(expectedResult, formattedStr); } @@ -217,9 +203,8 @@ public void testAuthorPUNC(String arg, String expectedResult) { "'Comma, Semicolon', 'B. C. Bruce, C. Manson, J. Jumper; C. Chuckles'", // Comma Semicolon }) public void testAuthorSEPARATORS(String arg, String expectedResult) { - ParamLayoutFormatter a = new Authors(); - a.setArgument(arg); - String formattedStr = a.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles"); + authorsLayoutFormatter.setArgument(arg); + String formattedStr = authorsLayoutFormatter.format("Bob Croydon Bruce and Charles Manson and Jolly Jumper and Chuck Chuckles"); assertEquals(expectedResult, formattedStr); } } diff --git a/src/test/java/org/jabref/logic/layout/format/FileLinkTest.java b/src/test/java/org/jabref/logic/layout/format/FileLinkTest.java index f15ae958dc2..bd5c519188a 100644 --- a/src/test/java/org/jabref/logic/layout/format/FileLinkTest.java +++ b/src/test/java/org/jabref/logic/layout/format/FileLinkTest.java @@ -11,53 +11,48 @@ public class FileLinkTest { private FileLinkPreferences prefs; + private ParamLayoutFormatter fileLinkLayoutFormatter; @BeforeEach public void setUp() throws Exception { prefs = mock(FileLinkPreferences.class); + fileLinkLayoutFormatter = new FileLink(prefs); } @Test - public void testEmpty() { - assertEquals("", new FileLink(prefs).format("")); + public void formatEmpty() { + assertEquals("", fileLinkLayoutFormatter.format("")); } @Test - public void testNull() { - assertEquals("", - new FileLink(prefs).format(null)); + public void formatNull() { + assertEquals("", fileLinkLayoutFormatter.format(null)); } @Test - public void testOnlyFilename() { - assertEquals("test.pdf", - new FileLink(prefs).format("test.pdf")); + public void formatOnlyFilename() { + assertEquals("test.pdf", fileLinkLayoutFormatter.format("test.pdf")); } @Test - public void testCompleteRecord() { - assertEquals("test.pdf", - new FileLink(prefs) - .format("paper:test.pdf:PDF")); + public void formatCompleteRecord() { + assertEquals("test.pdf", fileLinkLayoutFormatter.format("paper:test.pdf:PDF")); } @Test - public void testMultipleFiles() { - ParamLayoutFormatter a = new FileLink(prefs); - assertEquals("test.pdf", a.format("paper:test.pdf:PDF;presentation:pres.ppt:PPT")); + public void formatMultipleFiles() { + assertEquals("test.pdf", fileLinkLayoutFormatter.format("paper:test.pdf:PDF;presentation:pres.ppt:PPT")); } @Test - public void testMultipleFilesPick() { - ParamLayoutFormatter a = new FileLink(prefs); - a.setArgument("ppt"); - assertEquals("pres.ppt", a.format("paper:test.pdf:PDF;presentation:pres.ppt:PPT")); + public void formatMultipleFilesPick() { + fileLinkLayoutFormatter.setArgument("ppt"); + assertEquals("pres.ppt", fileLinkLayoutFormatter.format("paper:test.pdf:PDF;presentation:pres.ppt:PPT")); } @Test - public void testMultipleFilesPickNonExistant() { - ParamLayoutFormatter a = new FileLink(prefs); - a.setArgument("doc"); - assertEquals("", a.format("paper:test.pdf:PDF;presentation:pres.ppt:PPT")); + public void formatMultipleFilesPickNonExistant() { + fileLinkLayoutFormatter.setArgument("doc"); + assertEquals("", fileLinkLayoutFormatter.format("paper:test.pdf:PDF;presentation:pres.ppt:PPT")); } } diff --git a/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java b/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java index f0aa9836743..e586c6c8a57 100644 --- a/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java +++ b/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java @@ -2,39 +2,42 @@ import org.jabref.logic.layout.LayoutFormatter; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; public class FirstPageTest { + private LayoutFormatter firstPageLayoutFormatter; + + @BeforeEach + void setup() { + firstPageLayoutFormatter = new FirstPage(); + } + @Test - public void testFormatEmpty() { - LayoutFormatter a = new FirstPage(); - assertEquals("", a.format("")); + public void formatEmpty() { + assertEquals("", firstPageLayoutFormatter.format("")); } @Test - public void testFormatNull() { - LayoutFormatter a = new FirstPage(); - assertEquals("", a.format(null)); + public void formatNull() { + assertEquals("", firstPageLayoutFormatter.format(null)); } @Test - public void testFormatSinglePage() { - LayoutFormatter a = new FirstPage(); - assertEquals("345", a.format("345")); + public void formatSinglePage() { + assertEquals("345", firstPageLayoutFormatter.format("345")); } @Test - public void testFormatSingleDash() { - LayoutFormatter a = new FirstPage(); - assertEquals("345", a.format("345-350")); + public void formatSingleDash() { + assertEquals("345", firstPageLayoutFormatter.format("345-350")); } @Test - public void testFormatDoubleDash() { - LayoutFormatter a = new FirstPage(); - assertEquals("345", a.format("345--350")); + public void formatDoubleDash() { + assertEquals("345", firstPageLayoutFormatter.format("345--350")); } } From 40cfca6aec5da40711a05620d5cc14ca7b0b157c Mon Sep 17 00:00:00 2001 From: BShaq Date: Sun, 2 May 2021 21:26:41 +0200 Subject: [PATCH 04/10] fixed checkstyle issues --- .../java/org/jabref/logic/layout/format/DOICheckTest.java | 5 ++--- .../logic/layout/format/RemoveBracketsAddCommaTest.java | 5 ++--- .../logic/util/strings/StringLengthComparatorTest.java | 3 --- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java b/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java index 70001fbd2dc..f77625ad2ab 100644 --- a/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java +++ b/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java @@ -5,13 +5,12 @@ import org.jabref.logic.layout.LayoutFormatter; import org.junit.jupiter.api.BeforeEach; - -import static org.junit.jupiter.api.Assertions.assertEquals; - import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import static org.junit.jupiter.api.Assertions.assertEquals; + public class DOICheckTest { private LayoutFormatter layoutFormatter; diff --git a/src/test/java/org/jabref/logic/layout/format/RemoveBracketsAddCommaTest.java b/src/test/java/org/jabref/logic/layout/format/RemoveBracketsAddCommaTest.java index c760b666ae8..666958bb44a 100644 --- a/src/test/java/org/jabref/logic/layout/format/RemoveBracketsAddCommaTest.java +++ b/src/test/java/org/jabref/logic/layout/format/RemoveBracketsAddCommaTest.java @@ -5,13 +5,12 @@ import org.jabref.logic.layout.LayoutFormatter; import org.junit.jupiter.api.BeforeEach; - -import static org.junit.jupiter.api.Assertions.assertEquals; - import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import static org.junit.jupiter.api.Assertions.assertEquals; + public class RemoveBracketsAddCommaTest { private LayoutFormatter formatter; diff --git a/src/test/java/org/jabref/logic/util/strings/StringLengthComparatorTest.java b/src/test/java/org/jabref/logic/util/strings/StringLengthComparatorTest.java index deb965393fb..174b2de8238 100644 --- a/src/test/java/org/jabref/logic/util/strings/StringLengthComparatorTest.java +++ b/src/test/java/org/jabref/logic/util/strings/StringLengthComparatorTest.java @@ -2,10 +2,7 @@ import java.util.stream.Stream; -import org.jabref.logic.formatter.bibtexfields.NormalizePagesFormatter; - import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; From 64d6887b2381e46574a4f0c5fd4996824f67e331 Mon Sep 17 00:00:00 2001 From: BShaq <34344089+BShaq@users.noreply.github.com> Date: Mon, 3 May 2021 09:38:05 +0200 Subject: [PATCH 05/10] Moved setup up to field variable in AuthorsTest Co-authored-by: Oliver Kopp --- .../java/org/jabref/logic/layout/format/AuthorsTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java b/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java index 64f80c8c080..c28e90183fc 100644 --- a/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java +++ b/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java @@ -11,12 +11,7 @@ public class AuthorsTest { - private ParamLayoutFormatter authorsLayoutFormatter; - - @BeforeEach - void setup() { - authorsLayoutFormatter = new Authors(); - } + private ParamLayoutFormatter authorsLayoutFormatter = new Authors(); @Test public void testStandardUsage() { From d39820673fe62fc460f65922616fbce6fef685a6 Mon Sep 17 00:00:00 2001 From: BShaq <34344089+BShaq@users.noreply.github.com> Date: Mon, 3 May 2021 10:10:11 +0200 Subject: [PATCH 06/10] Moved setup up to field variable in FirstPageTest Co-authored-by: Oliver Kopp --- .../java/org/jabref/logic/layout/format/FirstPageTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java b/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java index e586c6c8a57..a20261286bc 100644 --- a/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java +++ b/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java @@ -9,12 +9,7 @@ public class FirstPageTest { - private LayoutFormatter firstPageLayoutFormatter; - - @BeforeEach - void setup() { - firstPageLayoutFormatter = new FirstPage(); - } + private LayoutFormatter firstPageLayoutFormatter = new FirstPage(); @Test public void formatEmpty() { From 792b9ab7f925dae689b3e2f185d8256e26b8fef6 Mon Sep 17 00:00:00 2001 From: BShaq Date: Mon, 3 May 2021 10:19:37 +0200 Subject: [PATCH 07/10] removed not used import in AuthorsTest --- src/test/java/org/jabref/logic/layout/format/AuthorsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java b/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java index c28e90183fc..8a3e22410c3 100644 --- a/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java +++ b/src/test/java/org/jabref/logic/layout/format/AuthorsTest.java @@ -2,7 +2,6 @@ import org.jabref.logic.layout.ParamLayoutFormatter; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; From 06d94410c1d6f46ef98092cfc75f3d0c8cddf8cd Mon Sep 17 00:00:00 2001 From: BShaq Date: Mon, 3 May 2021 10:20:35 +0200 Subject: [PATCH 08/10] Moved setup up to field variable in DOICheckTest --- .../java/org/jabref/logic/layout/format/DOICheckTest.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java b/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java index f77625ad2ab..2c7fb18598c 100644 --- a/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java +++ b/src/test/java/org/jabref/logic/layout/format/DOICheckTest.java @@ -4,7 +4,6 @@ import org.jabref.logic.layout.LayoutFormatter; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -13,12 +12,7 @@ public class DOICheckTest { - private LayoutFormatter layoutFormatter; - - @BeforeEach - void setup() { - layoutFormatter = new DOICheck(); - } + private LayoutFormatter layoutFormatter = new DOICheck(); @ParameterizedTest @MethodSource("provideDOI") From 021d595d367f891866aef7e650394174c14efe52 Mon Sep 17 00:00:00 2001 From: BShaq Date: Mon, 3 May 2021 10:21:29 +0200 Subject: [PATCH 09/10] refactored FileLinkTest into parameterized tests --- .../logic/layout/format/FileLinkTest.java | 60 ++++++++----------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/FileLinkTest.java b/src/test/java/org/jabref/logic/layout/format/FileLinkTest.java index bd5c519188a..7519473feb0 100644 --- a/src/test/java/org/jabref/logic/layout/format/FileLinkTest.java +++ b/src/test/java/org/jabref/logic/layout/format/FileLinkTest.java @@ -1,9 +1,13 @@ package org.jabref.logic.layout.format; +import java.util.stream.Stream; + import org.jabref.logic.layout.ParamLayoutFormatter; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.mock; @@ -19,40 +23,24 @@ public void setUp() throws Exception { fileLinkLayoutFormatter = new FileLink(prefs); } - @Test - public void formatEmpty() { - assertEquals("", fileLinkLayoutFormatter.format("")); - } - - @Test - public void formatNull() { - assertEquals("", fileLinkLayoutFormatter.format(null)); - } - - @Test - public void formatOnlyFilename() { - assertEquals("test.pdf", fileLinkLayoutFormatter.format("test.pdf")); - } - - @Test - public void formatCompleteRecord() { - assertEquals("test.pdf", fileLinkLayoutFormatter.format("paper:test.pdf:PDF")); - } - - @Test - public void formatMultipleFiles() { - assertEquals("test.pdf", fileLinkLayoutFormatter.format("paper:test.pdf:PDF;presentation:pres.ppt:PPT")); - } - - @Test - public void formatMultipleFilesPick() { - fileLinkLayoutFormatter.setArgument("ppt"); - assertEquals("pres.ppt", fileLinkLayoutFormatter.format("paper:test.pdf:PDF;presentation:pres.ppt:PPT")); - } - - @Test - public void formatMultipleFilesPickNonExistant() { - fileLinkLayoutFormatter.setArgument("doc"); - assertEquals("", fileLinkLayoutFormatter.format("paper:test.pdf:PDF;presentation:pres.ppt:PPT")); + @ParameterizedTest + @MethodSource("provideFileLinks") + void formatFileLinks(String formattedFileLink, String originalFileLink, String desiredDocType) { + if (!desiredDocType.isEmpty()) { + fileLinkLayoutFormatter.setArgument(desiredDocType); + } + assertEquals(formattedFileLink, fileLinkLayoutFormatter.format(originalFileLink)); + } + + private static Stream provideFileLinks() { + return Stream.of( + Arguments.of("", "", ""), + Arguments.of("", null, ""), + Arguments.of("test.pdf", "test.pdf", ""), + Arguments.of("test.pdf", "paper:test.pdf:PDF", ""), + Arguments.of("test.pdf", "paper:test.pdf:PDF;presentation:pres.ppt:PPT", ""), + Arguments.of("pres.ppt", "paper:test.pdf:PDF;presentation:pres.ppt:PPT", "ppt"), + Arguments.of("", "paper:test.pdf:PDF;presentation:pres.ppt:PPT", "doc") + ); } } From 79472d258874239b7a9a2d1215f762416941036d Mon Sep 17 00:00:00 2001 From: BShaq Date: Mon, 3 May 2021 10:21:58 +0200 Subject: [PATCH 10/10] refactored FirstPageTest into parameterized tests --- .../logic/layout/format/FirstPageTest.java | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java b/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java index a20261286bc..efe7f669151 100644 --- a/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java +++ b/src/test/java/org/jabref/logic/layout/format/FirstPageTest.java @@ -1,9 +1,12 @@ package org.jabref.logic.layout.format; +import java.util.stream.Stream; + import org.jabref.logic.layout.LayoutFormatter; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -11,28 +14,19 @@ public class FirstPageTest { private LayoutFormatter firstPageLayoutFormatter = new FirstPage(); - @Test - public void formatEmpty() { - assertEquals("", firstPageLayoutFormatter.format("")); - } - - @Test - public void formatNull() { - assertEquals("", firstPageLayoutFormatter.format(null)); - } - - @Test - public void formatSinglePage() { - assertEquals("345", firstPageLayoutFormatter.format("345")); - } - - @Test - public void formatSingleDash() { - assertEquals("345", firstPageLayoutFormatter.format("345-350")); + @ParameterizedTest + @MethodSource("providePages") + void formatPages(String formattedFirstPage, String originalPages) { + assertEquals(formattedFirstPage, firstPageLayoutFormatter.format(originalPages)); } - @Test - public void formatDoubleDash() { - assertEquals("345", firstPageLayoutFormatter.format("345--350")); + private static Stream providePages() { + return Stream.of( + Arguments.of("", ""), + Arguments.of("", null), + Arguments.of("345", "345"), + Arguments.of("345", "345-350"), + Arguments.of("345", "345--350") + ); } }