From 05363b1f5cde2011be44a85eaeef6bc911c48442 Mon Sep 17 00:00:00 2001 From: BShaq Date: Sun, 2 May 2021 17:36:28 +0200 Subject: [PATCH 1/5] refactoring of existing test classes Created parameterized tests for LastPageTest.java & NoSpaceBetweenAbbreviationsTest.jave Created setup() method to reduce test code duplication and renamed methods (removed 'test' prefix) --- .../logic/layout/format/LastPageTest.java | 52 +++++++--------- .../NoSpaceBetweenAbbreviationsTest.java | 36 ++++++++--- .../logic/layout/format/WrapContentTest.java | 62 +++++++++---------- 3 files changed, 81 insertions(+), 69 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/LastPageTest.java b/src/test/java/org/jabref/logic/layout/format/LastPageTest.java index 6647038a126..05d71274cd3 100644 --- a/src/test/java/org/jabref/logic/layout/format/LastPageTest.java +++ b/src/test/java/org/jabref/logic/layout/format/LastPageTest.java @@ -1,46 +1,40 @@ 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 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 LastPageTest { - @Test - public void testFormatEmpty() { - LayoutFormatter a = new LastPage(); - assertEquals("", a.format("")); - } + private LayoutFormatter lastPageLayoutFormatter; - @Test - public void testFormatNull() { - LayoutFormatter a = new LastPage(); - assertEquals("", a.format(null)); + @BeforeEach + void setup() { + lastPageLayoutFormatter = new LastPage(); } - @Test - public void testFormatSinglePage() { - LayoutFormatter a = new LastPage(); - assertEquals("345", a.format("345")); + @ParameterizedTest + @MethodSource("provideArguments") + void formatLastPage(String formattedText, String originalText) { + assertEquals(formattedText, lastPageLayoutFormatter.format(originalText)); } - @Test - public void testFormatSingleDash() { - LayoutFormatter a = new LastPage(); - assertEquals("350", a.format("345-350")); - } - - @Test - public void testFormatDoubleDash() { - LayoutFormatter a = new LastPage(); - assertEquals("350", a.format("345--350")); - } + private static Stream provideArguments() { + return Stream.of( + Arguments.of("", ""), + Arguments.of("", null), + Arguments.of("345", "345"), + Arguments.of("350", "345-350"), + Arguments.of("350", "345--350"), + Arguments.of("", "--") + ); - @Test - public void testFinalCoverageCase() { - LayoutFormatter a = new LastPage(); - assertEquals("", a.format("--")); } } diff --git a/src/test/java/org/jabref/logic/layout/format/NoSpaceBetweenAbbreviationsTest.java b/src/test/java/org/jabref/logic/layout/format/NoSpaceBetweenAbbreviationsTest.java index bd787b1ce39..a795123ebab 100644 --- a/src/test/java/org/jabref/logic/layout/format/NoSpaceBetweenAbbreviationsTest.java +++ b/src/test/java/org/jabref/logic/layout/format/NoSpaceBetweenAbbreviationsTest.java @@ -1,20 +1,38 @@ 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 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 NoSpaceBetweenAbbreviationsTest { - @Test - public void testFormat() { - LayoutFormatter f = new NoSpaceBetweenAbbreviations(); - assertEquals("", f.format("")); - assertEquals("John Meier", f.format("John Meier")); - assertEquals("J.F. Kennedy", f.format("J. F. Kennedy")); - assertEquals("J.R.R. Tolkien", f.format("J. R. R. Tolkien")); - assertEquals("J.R.R. Tolkien and J.F. Kennedy", f.format("J. R. R. Tolkien and J. F. Kennedy")); + private LayoutFormatter nsbaLayoutFormatter; + + @BeforeEach + void setup() { + nsbaLayoutFormatter = new NoSpaceBetweenAbbreviations(); + } + + @ParameterizedTest + @MethodSource("provideAbbreviations") + void formatAbbreviations(String formattedAbbreviation, String originalAbbreviation) { + assertEquals(formattedAbbreviation, nsbaLayoutFormatter.format(originalAbbreviation)); + } + + private static Stream provideAbbreviations() { + return Stream.of( + Arguments.of("", ""), + Arguments.of("John Meier", "John Meier"), + Arguments.of("J.F. Kennedy", "J. F. Kennedy"), + Arguments.of("J.R.R. Tolkien", "J. R. R. Tolkien"), + Arguments.of("J.R.R. Tolkien and J.F. Kennedy", "J. R. R. Tolkien and J. F. Kennedy") + ); } } diff --git a/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java b/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java index 6184b8ccfff..ae06bd1d216 100644 --- a/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java +++ b/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java @@ -2,64 +2,64 @@ import org.jabref.logic.layout.ParamLayoutFormatter; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; public class WrapContentTest { + private ParamLayoutFormatter wrapContentParamLayoutFormatter; + + @BeforeEach + void setup() { + wrapContentParamLayoutFormatter = new WrapContent(); + } + @Test - public void testSimpleText() { - ParamLayoutFormatter a = new WrapContent(); - a.setArgument("<,>"); - assertEquals("", a.format("Bob")); + public void formatSimpleText() { + wrapContentParamLayoutFormatter.setArgument("<,>"); + assertEquals("", wrapContentParamLayoutFormatter.format("Bob")); } @Test - public void testEmptyStart() { - ParamLayoutFormatter a = new WrapContent(); - a.setArgument(",:"); - assertEquals("Bob:", a.format("Bob")); + public void formatEmptyStart() { + wrapContentParamLayoutFormatter.setArgument(",:"); + assertEquals("Bob:", wrapContentParamLayoutFormatter.format("Bob")); } @Test - public void testEmptyEnd() { - ParamLayoutFormatter a = new WrapContent(); - a.setArgument("Content: ,"); - assertEquals("Content: Bob", a.format("Bob")); + public void formatEmptyEnd() { + wrapContentParamLayoutFormatter.setArgument("Content: ,"); + assertEquals("Content: Bob", wrapContentParamLayoutFormatter.format("Bob")); } @Test - public void testEscaping() { - ParamLayoutFormatter a = new WrapContent(); - a.setArgument("Name\\,Field\\,,\\,Author"); - assertEquals("Name,Field,Bob,Author", a.format("Bob")); + public void formatEscaping() { + wrapContentParamLayoutFormatter.setArgument("Name\\,Field\\,,\\,Author"); + assertEquals("Name,Field,Bob,Author", wrapContentParamLayoutFormatter.format("Bob")); } @Test - public void testFormatNullExpectNothingAdded() { - ParamLayoutFormatter a = new WrapContent(); - a.setArgument("Eds.,Ed."); - assertEquals(null, a.format(null)); + public void formatNull() { + wrapContentParamLayoutFormatter.setArgument("Eds.,Ed."); + assertEquals(null, wrapContentParamLayoutFormatter.format(null)); } @Test - public void testFormatEmptyExpectNothingAdded() { - ParamLayoutFormatter a = new WrapContent(); - a.setArgument("Eds.,Ed."); - assertEquals("", a.format("")); + public void formatEmptyString() { + wrapContentParamLayoutFormatter.setArgument("Eds.,Ed."); + assertEquals("", wrapContentParamLayoutFormatter.format("")); } @Test - public void testNoArgumentSetExpectNothingAdded() { - ParamLayoutFormatter a = new WrapContent(); - assertEquals("Bob Bruce and Jolly Jumper", a.format("Bob Bruce and Jolly Jumper")); + public void noArgumentSetInFormatter() { + assertEquals("Bob Bruce and Jolly Jumper", wrapContentParamLayoutFormatter.format("Bob Bruce and Jolly Jumper")); } @Test - public void testNoProperArgumentExpectNothingAdded() { - ParamLayoutFormatter a = new WrapContent(); - a.setArgument("Eds."); - assertEquals("Bob Bruce and Jolly Jumper", a.format("Bob Bruce and Jolly Jumper")); + public void formatNoProperArgumentSet() { + wrapContentParamLayoutFormatter.setArgument("Eds."); + assertEquals("Bob Bruce and Jolly Jumper", wrapContentParamLayoutFormatter.format("Bob Bruce and Jolly Jumper")); } } From 4b31270f10c2100244bb3c1856e4033faf5c795b Mon Sep 17 00:00:00 2001 From: BShaq Date: Sun, 2 May 2021 17:37:06 +0200 Subject: [PATCH 2/5] added boundary case for UnprotectTermsFormatterTest.java --- .../logic/formatter/casechanger/UnprotectTermsFormatterTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/jabref/logic/formatter/casechanger/UnprotectTermsFormatterTest.java b/src/test/java/org/jabref/logic/formatter/casechanger/UnprotectTermsFormatterTest.java index 28b78052958..06a285705bd 100644 --- a/src/test/java/org/jabref/logic/formatter/casechanger/UnprotectTermsFormatterTest.java +++ b/src/test/java/org/jabref/logic/formatter/casechanger/UnprotectTermsFormatterTest.java @@ -18,6 +18,7 @@ public class UnprotectTermsFormatterTest { private static Stream terms() throws IOException { return Stream.of( + Arguments.of("", ""), Arguments.of("VLSI", "{VLSI}"), Arguments.of("VLsI", "VLsI"), Arguments.of("VLSI", "VLSI"), From 288fe1fcea055cc5bd52f66df44c0834f482eab1 Mon Sep 17 00:00:00 2001 From: BShaq Date: Mon, 3 May 2021 11:14:00 +0200 Subject: [PATCH 3/5] move setup to field variable as suggested in #7687 --- .../java/org/jabref/logic/layout/format/LastPageTest.java | 8 +------- .../layout/format/NoSpaceBetweenAbbreviationsTest.java | 8 +------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/LastPageTest.java b/src/test/java/org/jabref/logic/layout/format/LastPageTest.java index 05d71274cd3..d18836a04c7 100644 --- a/src/test/java/org/jabref/logic/layout/format/LastPageTest.java +++ b/src/test/java/org/jabref/logic/layout/format/LastPageTest.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 LastPageTest { - private LayoutFormatter lastPageLayoutFormatter; - - @BeforeEach - void setup() { - lastPageLayoutFormatter = new LastPage(); - } + private LayoutFormatter lastPageLayoutFormatter = new LastPage(); @ParameterizedTest @MethodSource("provideArguments") diff --git a/src/test/java/org/jabref/logic/layout/format/NoSpaceBetweenAbbreviationsTest.java b/src/test/java/org/jabref/logic/layout/format/NoSpaceBetweenAbbreviationsTest.java index a795123ebab..7c75e579b86 100644 --- a/src/test/java/org/jabref/logic/layout/format/NoSpaceBetweenAbbreviationsTest.java +++ b/src/test/java/org/jabref/logic/layout/format/NoSpaceBetweenAbbreviationsTest.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 NoSpaceBetweenAbbreviationsTest { - private LayoutFormatter nsbaLayoutFormatter; - - @BeforeEach - void setup() { - nsbaLayoutFormatter = new NoSpaceBetweenAbbreviations(); - } + private LayoutFormatter nsbaLayoutFormatter = new NoSpaceBetweenAbbreviations(); @ParameterizedTest @MethodSource("provideAbbreviations") From 0a628c07e2e73e0ef1872c55be63f61c89961459 Mon Sep 17 00:00:00 2001 From: BShaq Date: Mon, 3 May 2021 11:32:31 +0200 Subject: [PATCH 4/5] moved setup up to field variable and converted test class to parameterized test --- .../logic/layout/format/WrapContentTest.java | 82 +++++++------------ 1 file changed, 28 insertions(+), 54 deletions(-) diff --git a/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java b/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java index ae06bd1d216..36b7e9f528f 100644 --- a/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java +++ b/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java @@ -1,65 +1,39 @@ 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; public class WrapContentTest { - private ParamLayoutFormatter wrapContentParamLayoutFormatter; - - @BeforeEach - void setup() { - wrapContentParamLayoutFormatter = new WrapContent(); - } - - @Test - public void formatSimpleText() { - wrapContentParamLayoutFormatter.setArgument("<,>"); - assertEquals("", wrapContentParamLayoutFormatter.format("Bob")); - } - - @Test - public void formatEmptyStart() { - wrapContentParamLayoutFormatter.setArgument(",:"); - assertEquals("Bob:", wrapContentParamLayoutFormatter.format("Bob")); - } - - @Test - public void formatEmptyEnd() { - wrapContentParamLayoutFormatter.setArgument("Content: ,"); - assertEquals("Content: Bob", wrapContentParamLayoutFormatter.format("Bob")); - } - - @Test - public void formatEscaping() { - wrapContentParamLayoutFormatter.setArgument("Name\\,Field\\,,\\,Author"); - assertEquals("Name,Field,Bob,Author", wrapContentParamLayoutFormatter.format("Bob")); - } - - @Test - public void formatNull() { - wrapContentParamLayoutFormatter.setArgument("Eds.,Ed."); - assertEquals(null, wrapContentParamLayoutFormatter.format(null)); - } - - @Test - public void formatEmptyString() { - wrapContentParamLayoutFormatter.setArgument("Eds.,Ed."); - assertEquals("", wrapContentParamLayoutFormatter.format("")); - } - - @Test - public void noArgumentSetInFormatter() { - assertEquals("Bob Bruce and Jolly Jumper", wrapContentParamLayoutFormatter.format("Bob Bruce and Jolly Jumper")); - } - - @Test - public void formatNoProperArgumentSet() { - wrapContentParamLayoutFormatter.setArgument("Eds."); - assertEquals("Bob Bruce and Jolly Jumper", wrapContentParamLayoutFormatter.format("Bob Bruce and Jolly Jumper")); + private ParamLayoutFormatter wrapContentParamLayoutFormatter = new WrapContent(); + + @ParameterizedTest + @MethodSource("provideContent") + void formatContent(String formattedContent, String originalContent, String desiredFormat) { + if(!desiredFormat.isEmpty()) { + wrapContentParamLayoutFormatter.setArgument(desiredFormat); + } + + assertEquals(formattedContent, wrapContentParamLayoutFormatter.format(originalContent)); + } + + private static Stream provideContent() { + return Stream.of( + Arguments.of("", "Bob", "<,>"), + Arguments.of("Bob:", "Bob", ",:"), + Arguments.of("Content: Bob", "Bob", "Content: ,"), + Arguments.of("Name,Field,Bob,Author", "Bob", "Name\\,Field\\,,\\,Author"), + Arguments.of(null, null, "Eds.,Ed."), + Arguments.of("", "", "Eds.,Ed."), + Arguments.of("Bob Bruce and Jolly Jumper", "Bob Bruce and Jolly Jumper", ""), + Arguments.of("Bob Bruce and Jolly Jumper", "Bob Bruce and Jolly Jumper", "Eds.") + ); } } From 06192ea037448ebe9f637efaa626c55143928c7e Mon Sep 17 00:00:00 2001 From: BShaq Date: Mon, 3 May 2021 11:39:00 +0200 Subject: [PATCH 5/5] fixed checkstyle issue --- .../java/org/jabref/logic/layout/format/WrapContentTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java b/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java index 36b7e9f528f..bb933c44b5c 100644 --- a/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java +++ b/src/test/java/org/jabref/logic/layout/format/WrapContentTest.java @@ -17,7 +17,7 @@ public class WrapContentTest { @ParameterizedTest @MethodSource("provideContent") void formatContent(String formattedContent, String originalContent, String desiredFormat) { - if(!desiredFormat.isEmpty()) { + if (!desiredFormat.isEmpty()) { wrapContentParamLayoutFormatter.setArgument(desiredFormat); }