-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Refactor simple Unit Tests #7571
Changes from 8 commits
93a97bc
9c62748
4d11f5f
5ef25ef
e9aa9b3
3563a1b
7e4a486
98004a9
94d4625
50420b9
a23e989
a30c0d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,34 @@ | ||
package org.jabref.logic.bst; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import java.util.stream.Stream; | ||
|
||
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.junit.jupiter.api.Assertions.fail; | ||
|
||
public class BibtexPurifyTest { | ||
|
||
@Test | ||
public void testPurify() { | ||
assertPurify("i", "i"); | ||
assertPurify("0I ", "0I~ "); | ||
assertPurify("Hi Hi ", "Hi Hi "); | ||
assertPurify("oe", "{\\oe}"); | ||
assertPurify("Hi oeHi ", "Hi {\\oe }Hi "); | ||
assertPurify("Jonathan Meyer and Charles Louis Xavier Joseph de la Vallee Poussin", "Jonathan Meyer and Charles Louis Xavier Joseph de la Vall{\\'e}e Poussin"); | ||
assertPurify("e", "{\\'e}"); | ||
assertPurify("Edouard Masterly", "{\\'{E}}douard Masterly"); | ||
assertPurify("Ulrich Underwood and Ned Net and Paul Pot", "Ulrich {\\\"{U}}nderwood and Ned {\\~N}et and Paul {\\={P}}ot"); | ||
@ParameterizedTest | ||
@MethodSource("provideTestStrings") | ||
public void testPurify(String expected, String toBePurified) { | ||
assertPurify(expected, toBePurified); | ||
} | ||
|
||
private static Stream<Arguments> provideTestStrings() { | ||
return Stream.of( | ||
Arguments.of("i", "i"), | ||
Arguments.of("0I ", "0I~ "), | ||
Arguments.of("Hi Hi ", "Hi Hi "), | ||
Arguments.of("oe", "{\\oe}"), | ||
Arguments.of("Hi oeHi ", "Hi {\\oe }Hi "), | ||
Arguments.of("Jonathan Meyer and Charles Louis Xavier Joseph de la Vallee Poussin", "Jonathan Meyer and Charles Louis Xavier Joseph de la Vall{\\'e}e Poussin"), | ||
Arguments.of("e", "{\\'e}"), | ||
Arguments.of("Edouard Masterly", "{\\'{E}}douard Masterly"), | ||
Arguments.of("Ulrich Underwood and Ned Net and Paul Pot", "Ulrich {\\\"{U}}nderwood and Ned {\\~N}et and Paul {\\={P}}ot") | ||
); | ||
} | ||
|
||
private void assertPurify(final String string, final String string2) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please integrate in |
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,5 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
@@ -10,12 +9,7 @@ | |||||
*/ | ||||||
class AddBracesFormatterTest { | ||||||
|
||||||
private AddBracesFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
public void setUp() { | ||||||
formatter = new AddBracesFormatter(); | ||||||
} | ||||||
private static final AddBracesFormatter formatter = new AddBracesFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nononono static
Suggested change
|
||||||
|
||||||
@Test | ||||||
public void formatAddsSingleEnclosingBraces() { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,5 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
@@ -10,12 +9,7 @@ | |||||
*/ | ||||||
class CleanupUrlFormatterTest { | ||||||
|
||||||
private CleanupUrlFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
void setUp() { | ||||||
formatter = new CleanupUrlFormatter(); | ||||||
} | ||||||
private static final CleanupUrlFormatter formatter = new CleanupUrlFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
@Test | ||||||
void removeSpecialSymbolsFromURLLink() { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,5 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
@@ -10,12 +9,7 @@ | |||||
*/ | ||||||
public class ClearFormatterTest { | ||||||
|
||||||
private ClearFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
public void setUp() { | ||||||
formatter = new ClearFormatter(); | ||||||
} | ||||||
private static final ClearFormatter formatter = new ClearFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
/** | ||||||
* Check whether the clear formatter really returns the empty string for the empty string | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,18 +1,12 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
||||||
class EscapeAmpersandsFormatterTest { | ||||||
|
||||||
private EscapeAmpersandsFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
void setUp() { | ||||||
formatter = new EscapeAmpersandsFormatter(); | ||||||
} | ||||||
private static final EscapeAmpersandsFormatter formatter = new EscapeAmpersandsFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
@Test | ||||||
void formatReturnsSameTextIfNoAmpersandsPresent() throws Exception { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,18 +1,12 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
||||||
class EscapeUnderscoresFormatterTest { | ||||||
|
||||||
private EscapeUnderscoresFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
void setUp() { | ||||||
formatter = new EscapeUnderscoresFormatter(); | ||||||
} | ||||||
private static final EscapeUnderscoresFormatter formatter = new EscapeUnderscoresFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
@Test | ||||||
void formatReturnsSameTextIfNoUnderscoresPresent() throws Exception { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,5 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
@@ -10,12 +9,7 @@ | |||||
*/ | ||||||
public class HtmlToLatexFormatterTest { | ||||||
|
||||||
private HtmlToLatexFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
public void setUp() { | ||||||
formatter = new HtmlToLatexFormatter(); | ||||||
} | ||||||
private static final HtmlToLatexFormatter formatter = new HtmlToLatexFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
@Test | ||||||
public void formatWithoutHtmlCharactersReturnsSameString() { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,18 +1,12 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
||||||
public class HtmlToUnicodeFormatterTest { | ||||||
|
||||||
private HtmlToUnicodeFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
public void setUp() { | ||||||
formatter = new HtmlToUnicodeFormatter(); | ||||||
} | ||||||
private static final HtmlToUnicodeFormatter formatter = new HtmlToUnicodeFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
@Test | ||||||
public void formatWithoutHtmlCharactersReturnsSameString() { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,18 +1,12 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
||||||
class LatexCleanupFormatterTest { | ||||||
|
||||||
private LatexCleanupFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
void setUp() { | ||||||
formatter = new LatexCleanupFormatter(); | ||||||
} | ||||||
private static final LatexCleanupFormatter formatter = new LatexCleanupFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
@Test | ||||||
void test() { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,5 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
@@ -10,12 +9,7 @@ | |||||
*/ | ||||||
public class NormalizeDateFormatterTest { | ||||||
|
||||||
private NormalizeDateFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
public void setUp() { | ||||||
formatter = new NormalizeDateFormatter(); | ||||||
} | ||||||
private static final NormalizeDateFormatter formatter = new NormalizeDateFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
@Test | ||||||
public void formatDateYYYYMM0D() { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,5 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
@@ -10,12 +9,7 @@ | |||||
*/ | ||||||
public class NormalizeEnDashesFormatterTest { | ||||||
|
||||||
private NormalizeEnDashesFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
public void setUp() { | ||||||
formatter = new NormalizeEnDashesFormatter(); | ||||||
} | ||||||
private static final NormalizeEnDashesFormatter formatter = new NormalizeEnDashesFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
@Test | ||||||
public void formatExample() { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,5 @@ | ||||||
package org.jabref.logic.formatter.bibtexfields; | ||||||
|
||||||
import org.junit.jupiter.api.BeforeEach; | ||||||
import org.junit.jupiter.api.Test; | ||||||
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
|
@@ -10,12 +9,7 @@ | |||||
*/ | ||||||
public class NormalizeMonthFormatterTest { | ||||||
|
||||||
private NormalizeMonthFormatter formatter; | ||||||
|
||||||
@BeforeEach | ||||||
public void setUp() { | ||||||
formatter = new NormalizeMonthFormatter(); | ||||||
} | ||||||
private static final NormalizeMonthFormatter formatter = new NormalizeMonthFormatter(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
@Test | ||||||
public void formatExample() { | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change this? I think it's good practice to initialize the object under test in a setup method, so that e.g. changes to it in one test doesn't leak to other tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I see it correctly it is not possible to change the CrossRefEntryComparator Object after creation. In other words there are no setter methods and the constructor does not take any arguments.
That is why I think the beforeeach is superfluous. Why create a new object, if the one created can be used?
On the other hand, I get your point. If changes to this class are done, like adding arguments to the constructor or adding setter methods, then the BeforeEach makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creation of objects I java is extremely efficient. This shouldn't be a concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In tests, one just does not use
static
.setup
just does that) --> move the initialization code up.setup
(as in the code)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reason for no static: Each test has to be independed of the last test. Thus, everything under test has to be in the initial state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! I'll revert these changes then and use the setup method for those formatter tests.