Add simple Unit Tests for #6207 #6240

dimitra-karadima · 2020-04-04T16:50:38Z

I have split some Unit Tests into different methods with only one assert statement.
@koppor Please let me know what you think about my additions so far!

Change in CHANGELOG.md described (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for bigger UI changes)
Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

Add the following changes: -remove multiple assert statements in test cases -split each assert statement in different test methods with meaningful test names

Fix some encoding problems Fix merge conflict with the master branch

Siedlerchr · 2020-04-04T17:31:36Z

I would split it even further, create a separate class for each IntegrityCheckTest (e.g. one class for Edition, one for Author, ...) as we also have a different class for each integrity check

koppor · 2020-04-05T12:45:03Z

src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java

@@ -186,6 +292,17 @@ void testTitleChecks() {
        assertCorrect(withMode(createContext(StandardField.TITLE, "This is a {Title}"), BibDatabaseMode.BIBTEX));
        assertCorrect(withMode(createContext(StandardField.TITLE, "{C}urrent {C}hronicle"), BibDatabaseMode.BIBTEX));
        assertCorrect(withMode(createContext(StandardField.TITLE, "{A Model-Driven Approach for Monitoring {ebBP} BusinessTransactions}"), BibDatabaseMode.BIBTEX));
+        assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: This is a sub title 2"), BibDatabaseMode.BIBTEX));


I would move all these tests to TitleCheckerTest and just keep one subtitle tests here.

This test class here is only for an integration test (TitleChecker is integrated in the IntegrityCheck) - the "real" functionality is in org.jabref.logic.integrity.TitleChecker.

When you are on it, you could fix the capitalization in TitleCheckerTest:

calixtus · 2020-04-05T16:57:29Z

Hi @dimitra-karadima , thanks for you effort. Writing unit tests is sometimes boring, but can also be very educational. Thank you very much.
Also I wanted to ask you to keep the checkboxes in the PR description, as they are always useful to us to get a quick overview to which extent a PR is done. Even if only one checkbox makes sense in a PR.
I took the liberty to readd them by myself.

dimitra-karadima · 2020-04-05T17:28:28Z

Hi @dimitra-karadima , thanks for you effort. Writing unit tests is sometimes boring, but can also be very educational. Thank you very much.
Also I wanted to ask you to keep the checkboxes in the PR description, as they are always useful to us to get a quick overview to which extent a PR is done. Even if only one checkbox makes sense in a PR.
I took the liberty to readd them by myself.

Thanks for your input! I will keep it in mind for the upcoming PRs!

tobiasdiez · 2020-04-10T14:42:40Z

@dimitra-karadima Did you already found the time to address the comments above?

dimitra-karadima · 2020-04-10T19:55:49Z

@dimitra-karadima Did you already found the time to address the comments above?

@tobiasdiez Sorry I was a little bit busy but until Sunday I will commit my new changes!

Add the following changes: -move tests from IntegrityCheckTest to the appropriate Test classes -remove multiple assert statements in different test methods -fix the capitalization in TitleCheckerTest

Change two failing tests to succesfull Remove one test

dimitra-karadima · 2020-04-14T08:47:59Z

@dimitra-karadima Did you already found the time to address the comments above?

@tobiasdiez what do you think about my latest commit?

koppor

Thank you for continuing working on it.

Please use checker.checkValue instead of IntegrityCheckTest.... Reason: Keep the tests focused in the test subject. See the name "AbbreviationCheckerTest"? It tests the class "AbbreviationChecker".

Could you please rework the added tests to use that? You can work on a direct field value, no need to think of StandardField....

See other test classes in the same package for examples. E.g., https://github.com/JabRef/jabref/blob/master/src/test/java/org/jabref/logic/integrity/TitleCheckerTest.java

koppor · 2020-04-22T04:26:57Z

src/test/java/org/jabref/logic/integrity/ASCIICharacterCheckerTest.java

+public class ASCIICharacterCheckerTest {
+
+    @Test
+    void fieldAcceptsASCIICharacters() {


Suggested change

void fieldAcceptsASCIICharacters() {

void fieldAcceptsAsciiCharacters() {

Even not conform with current JabRef classes, please use defined camel case - see https://google.github.io/styleguide/javaguide.html#s5.3-camel-case for details.

koppor · 2020-04-22T04:29:02Z

src/test/java/org/jabref/logic/integrity/AbbreviationCheckerTest.java

+    @Test
+    void journalNameAcceptsFullForm() {
+        for (Field field : Arrays.asList(StandardField.BOOKTITLE, StandardField.JOURNAL)) {
+            IntegrityCheckTest.assertCorrect(IntegrityCheckTest.createContext(field, "IEEE Software"));


Please use checker.checkValue instead of IntegrityCheckTest.... Reason: Keep the tests focused in the test subject.

See the name "AbbreviationCheckerTest"? It tests the class "AbbreviationChecker". This class is instantiated in line 25.

Also applies for the other checks you added. Could you please rework?

@koppor I changed every class that has a checkValue method. But some of them like ASCIICharacterChecker.java has a different method(public List check(BibEntry entry)). Do you want me to change those tests too? If yes can you suggest me something? Because it is like IntegrityCheck.java and you haven't made any comments on my test class.

No errors: assertEquals(Collections.emptyList(), assciCharacterChecker.check(entry));

Errors: assertEquals(List.of(new IntegrityCheckMessage("...text", StandardFields.DOI)), assciCharacterChecker.check(entry));). This way, the real error message is checked. And if someone changes something in the checker, it is also noted in the tests. See our long disucssions at Remove dash from illegal characters. #6300 (which are also caused by failing tests - and we were forced to think about the intended behavior more deeply).

Add the following changes: -Use checker.checkValue or checker.check instead of IntegrityCheckTest... -Split a method with multiple assertion methods in FileCheckerTest into three different with a simple assertion method -Rename a method in ASCIICharacterCheckerTest in order to use defined camel case

koppor · 2020-05-05T18:58:34Z

Looks got o me. Please resolve the conflicts at IntegrityCheckTest.java.

dimitra-karadima · 2020-05-05T19:29:17Z

Looks got o me. Please resolve the conflicts at IntegrityCheckTest.java.

@koppor I fixed it!

koppor

Huge diff. Difficult to review. Think, I found the important parts.

Something went wrong with the merge. Cannot look now. Can you check it?

koppor · 2020-05-06T23:02:05Z

src/test/java/org/jabref/logic/integrity/BooktitleCheckerTest.java

+
+    @BeforeEach
+    public void setUp() {
+        checker = new BooktitleChecker();


No need for the @BeforeEach here. Just initialize the checker in line 13. --> One SLOC insted of 6 for the same functionality. -- JUnit initializes the class on each run.

Please check the other (modified) classes, too.

src/test/java/org/jabref/logic/integrity/EditionCheckerTest.java

koppor · 2020-05-06T23:03:24Z

src/test/java/org/jabref/logic/integrity/FileCheckerTest.java

+    private BibDatabaseContext bibDatabaseContext;
+    private List<IntegrityMessage> messages;
+    MetaData metaData = mock(MetaData.class);
+    //private FilePreferences f;


Why is here commented code?

koppor · 2020-05-06T23:05:33Z

src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java

@@ -390,21 +84,21 @@ private BibDatabaseContext createContext(Field field, String value, EntryType ty
        return new BibDatabaseContext(bibDatabase);
    }

-    private BibDatabaseContext createContext(Field field, String value, MetaData metaData) {
+    protected static BibDatabaseContext createContext(Field field, String value, MetaData metaData) {


Why was the modifier changed? Is this method used outside of this class? It should be private. Adding static is OK for me.

@koppor yes it was used outside of the class. I changed it now so all classes are independent. And I think I covered all the issues mentioned with my new commits. Please check it out and let me know what you think!

koppor · 2020-05-06T23:07:58Z

src/test/java/org/jabref/logic/integrity/FileCheckerTest.java

+
+    @BeforeEach
+    void setUp() {
+        //hash.put(StandardField.ABSTRACT, "");


I don't find this commented code in the old IntegrityCheck Test. Why is it here?

koppor · 2020-05-06T23:13:04Z

src/test/java/org/jabref/logic/integrity/FileCheckerTest.java

+        //hash.put(StandardField.ABSTRACT, "");
+        //f = new FilePreferences("", hash, true, "", "");
+        bibDatabaseContext = new BibDatabaseContext();
+        checker = new FileChecker(bibDatabaseContext, mock(FilePreferences.class));


Is this checker ever used in this class?

I would propose to

a) remove this unused code
b) readd the test void testFileChecks() { in the IntegrityCheck class as the DIFFERENCE to this test is that the FILE field is checked - and this coudl have some side effects.

More background:

I wondered why metaData has to be mocked. The only reason can be the special treatment of the file field. I think, we should keep this code. The code can be left in the IntegrityCheckerTest class - if we don't come up with some better place.

Add the following changes: -Revert all methods from protected static to private -Change BibtexKeyCheckerTest in order not to use IntegrityCheckTest's methods -Add two tests methods that were moved to another class

Due to recent merges to master the JournalAbbreviationRepository constructor changed so the creation of the object changes as well.

Remove it wherever it was just creating objects

Remove unnecessary BibEntry initialization and import

koppor · 2020-05-08T22:35:48Z

@dimitra-karadima Thank you for continuing to work on it. It would be very nice if you looked at our automated tests. The checkstyle is failing:

You can avoid the unused imports if you format the file. Please ensure that you have the JabRef code style active in IntelliJ. See https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace#using-jabrefs-code-style for details.

koppor

I just did not find the for loops. I checked the other tests - and I found them all re-added.

Please fix checkstyle, too.

Then, it should be good to go 🎉

koppor · 2020-05-08T22:38:57Z

src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java

-
-    @Test
-    void testAuthorNameChecks() {
-        for (Field field : FieldFactory.getPersonNameFields()) {


I don't find this for loop in the new code. Was it deleted completely or did I just not find it? If it was deleted, please re-add it. -- Maybe: Is the test already existing somewhere else?

@koppor I placed these tests in the PersonNamesCheckerTest. Do you want me to add the for loops in all the added tests there?

I just checked. Did not find assertCorrect(withMode(createContext(field, ""), BibDatabaseMode.BIBLATEX)) there. Could you do a double check for commit and push, please?

For the for loop, please use @MethodSource as described at https://www.baeldung.com/parameterized-tests-junit-5#6-method.

@koppor I will use the @MethodSource as you suggested but the line you mentioned have been replaced because the methods used are declared in the IntegrityCheckTest and we want all the test classes to be interdependent.

Good step forward. Note to self:

Check for FieldFactory.getPersonNameFields() in PersonNamesCheckerTest.

Check for Stream.of(Arguments.of in the new code (as this was linked to from https://www.baeldung.com/parameterized-tests-junit-5#6-method)

After thinking longer, the first aspect cannot be made in PersonChecker.

Thus, please rewrite the test here to provide a Field from FieldFactory.getPersonNameFields() as source: Split the asserts up and have field as paramter. Thus, each assertCorrect and each assertWrong will become a separate @Test.

(The alternative is to provide two parameters to a @Test as you did in PersonNamesCheckerTest).

koppor · 2020-05-08T22:39:53Z

src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java

    }

    @Test
-    void testAbbreviationChecks() {
-        for (Field field : Arrays.asList(StandardField.BOOKTITLE, StandardField.JOURNAL)) {


I did not find this test in the diff. Is the test already existing somewhere else?

@koppor I thought that these tests were covered in the AbbreviationCheckerTest. Do you want me to re-add these tests here?

Rework test class because checkstyle is failing

koppor · 2020-05-11T18:20:09Z

Had to retrigger the build workflows as they were not executed correctly. Checkstyle still fails:

@dimitra-karadima You can execute the checkstyle checks locally. Either in the IDE or using gradlew checkstyleMain.

koppor · 2020-05-11T18:24:48Z

src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java

-
-    @Test
-    void testAuthorNameChecks() {
-        for (Field field : FieldFactory.getPersonNameFields()) {


I just checked. Did not find assertCorrect(withMode(createContext(field, ""), BibDatabaseMode.BIBLATEX)) there. Could you do a double check for commit and push, please?

For the for loop, please use @MethodSource as described at https://www.baeldung.com/parameterized-tests-junit-5#6-method.

Fix because instance variable definition was in wrong order.

Change from multiple test methods to two parameterized with @MethodSource

koppor

Thank you for the quick answer. I understood the intion of IntegrityCheckTest now more. Added a suggestion to the JavaDoc and commented the proposed changes accordingly. Could you please adress them?

koppor · 2020-05-12T04:27:37Z

src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java

-
-    @Test
-    void testAuthorNameChecks() {
-        for (Field field : FieldFactory.getPersonNameFields()) {


Good step forward. Note to self:

Check for FieldFactory.getPersonNameFields() in PersonNamesCheckerTest.

Check for Stream.of(Arguments.of in the new code (as this was linked to from https://www.baeldung.com/parameterized-tests-junit-5#6-method)

After thinking longer, the first aspect cannot be made in PersonChecker.

Thus, please rewrite the test here to provide a Field from FieldFactory.getPersonNameFields() as source: Split the asserts up and have field as paramter. Thus, each assertCorrect and each assertWrong will become a separate @Test.

(The alternative is to provide two parameters to a @Test as you did in PersonNamesCheckerTest).

koppor · 2020-05-12T04:28:46Z

src/test/java/org/jabref/logic/integrity/PersonNamesCheckerTest.java

@@ -62,4 +69,29 @@ public void validCorporateNameAndPerson() throws Exception {
        assertEquals(Optional.empty(), checker.checkValue("{JabRef} and Stefan Kolb"));
        assertEquals(Optional.empty(), checker.checkValue("{JabRef} and Kolb, Stefan"));
    }
+
+    @ParameterizedTest
+    @MethodSource("provideCorrectFormats")


This is a good step forward.

The aspect of a field check is missing. To keep that in IntegrityCheckTest is a good idea. See my comments there.

The inputs will be duplicated, but that is OK.

koppor · 2020-05-12T04:54:27Z

src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java

@@ -39,197 +37,23 @@
 class IntegrityCheckTest {


I think, a JavaDoc should be added explaning the difference between this test and the CheckerTests. Here a proposal:

Suggested change

class IntegrityCheckTest {

/**

* This class tests the Integrity Checker as a whole. Aspects are: selected fields, issues arising in a complete BibTeX entry, ... When testing a checker works with a certain input, this test has to go to a test belonging to the respective checker. See PersonNamesCheckerTest for an example test.

*/

class IntegrityCheckTest {

koppor · 2020-05-12T04:57:20Z

src/test/java/org/jabref/logic/integrity/PersonNamesCheckerTest.java

+    }
+
+    private static String[] provideCorrectFormats() {
+        String[] data = new String[] {"", "Knuth", "Donald E. Knuth and Kurt Cobain and A. Einstein"};


Since a single String string is required at each test, please use following code:

Suggested change

String[] data = new String[] {"", "Knuth", "Donald E. Knuth and Kurt Cobain and A. Einstein"};

return Stream.of("", "Knuth", "Donald E. Knuth and Kurt Cobain and A. Einstein");

This is more readble than String[]. The return type will be Stream<String>.

koppor · 2020-05-12T04:57:56Z

src/test/java/org/jabref/logic/integrity/PersonNamesCheckerTest.java

+    }
+
+    private static String[] provideIncorrectFormats() {
+        String[] data = new String[] {"   Knuth, Donald E. ", "Knuth, Donald E. and Kurt Cobain and A. Einstein",


See above --> Stream.of(...).

Change the return type of two methods from String[] to Stream<String> since it is more readable.

Add the following changes: -Add Javadoc -Rewrite testAuthorNameChecks with @MethodSource

koppor · 2020-05-13T05:09:58Z

Thank you for the follow up - I merged it as a single commit into a local branch in this repository. I will address the minor comments for myself there.

koppor

Only the empty line was missing. Fixed it for myself and will merge. Thank you for the work.

koppor · 2020-05-13T05:05:52Z

src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java

+ * Aspects are: selected fields, issues arising in a complete BibTeX entry, ... When testing a checker works with a certain input,
+ * this test has to go to a test belonging to the respective checker. See PersonNamesCheckerTest for an example test.
+ */
+


No empty line between JavaDoc and code.

* upstream/master: (50 commits) Keep group pane size when resizing window (#6180) (#6423) Changelog: Fix missing citation for biblatex-mla Update AUTHORS Check duplicate DOI (#6333) Fix missing citation for biblatex-mla Change EasyBind dependency (#6480) Add testing of latest dev version as mandatory Squashed 'src/main/resources/csl-styles/' changes from 5dad23d..586e0b8 Fix libre office connection and other progress dialogs (#6478) Fix clear year and month field when converting to biblatex (#6434) Add truncate as a BibTex key modifier (#6427) Add new authors (not all - they need more work) Remove empty line Add simple Unit Tests for #6207 (#6240) Enforce LeftCurly rule (#6452) Implement task progress indicator (and dialog) in the toolbar (#6443) Consider empty brackets Changelog update Added a test Fixed brackets in regular expressions ...

80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581) b6fb00e415 Create arachnologische-mitteilungen.csl (#6375) 2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580) fd230a7073 Create veterinary-clinical-pathology.csl (#6372) ac0afa3cae Create dedicated Basque APA file (#6370) fee0677a88 Update chicago-author-date.csl (#6289) ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230) f4116db325 Create Turcica.csl (#6240) 0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211) e73bf46674 Update vancouver.csl (#6156) b73c3ef193 Create independent TF AIP Style befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181) 048c9bddbc Add csl for SDMI (#6129) 1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567) b77084255f Update the-quarterly-journal-of-economics.csl (#6572) cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577) 9e9e08c219 Update isara-iso-690.csl (#6578) 4b4e8f442d Create silva-fennica.csl (#6568) dd8760bb2b Update american-journal-of-botany.csl (#6569) 8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560) 016050c4b7 Update geographie-et-cultures.csl (#6563) e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 80b3861bce121a64d43ef167581f8d100a4f70aa

…ce (#10048) 80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581) b6fb00e415 Create arachnologische-mitteilungen.csl (#6375) 2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580) fd230a7073 Create veterinary-clinical-pathology.csl (#6372) ac0afa3cae Create dedicated Basque APA file (#6370) fee0677a88 Update chicago-author-date.csl (#6289) ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230) f4116db325 Create Turcica.csl (#6240) 0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211) e73bf46674 Update vancouver.csl (#6156) b73c3ef193 Create independent TF AIP Style befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181) 048c9bddbc Add csl for SDMI (#6129) 1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567) b77084255f Update the-quarterly-journal-of-economics.csl (#6572) cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577) 9e9e08c219 Update isara-iso-690.csl (#6578) 4b4e8f442d Create silva-fennica.csl (#6568) dd8760bb2b Update american-journal-of-botany.csl (#6569) 8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560) 016050c4b7 Update geographie-et-cultures.csl (#6563) e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 80b3861bce121a64d43ef167581f8d100a4f70aa Co-authored-by: github actions <jabrefmail+webfeedback@gmail.com>

a97dbb32c5 Update studii-teologice.csl (#6591) e19e08780e Update acm-sig-proceedings-long-author-list.csl (#6594) c8abbcdd88 Update acm-sig-proceedings.csl (#6595) 725ace4a40 Create uppsala-university-library-harvard.csl (#6574) a973041e0e update bluebook-law-review.csl (#6583) 0891cfc49a Update masarykova-univerzita-pravnicka-fakulta.csl (#6589) 80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581) b6fb00e415 Create arachnologische-mitteilungen.csl (#6375) 2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580) fd230a7073 Create veterinary-clinical-pathology.csl (#6372) ac0afa3cae Create dedicated Basque APA file (#6370) fee0677a88 Update chicago-author-date.csl (#6289) ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230) f4116db325 Create Turcica.csl (#6240) 0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211) e73bf46674 Update vancouver.csl (#6156) b73c3ef193 Create independent TF AIP Style befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181) 048c9bddbc Add csl for SDMI (#6129) 1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567) b77084255f Update the-quarterly-journal-of-economics.csl (#6572) cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577) 9e9e08c219 Update isara-iso-690.csl (#6578) 4b4e8f442d Create silva-fennica.csl (#6568) dd8760bb2b Update american-journal-of-botany.csl (#6569) 8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560) 016050c4b7 Update geographie-et-cultures.csl (#6563) e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: a97dbb32c57c8c00e47422dae4ba4f480e753da5

0749a19b83 Update journal-of-experimental-botany.csl (#6604) b1768724fe Update modern-language-association.csl (#6606) dd364c1815 Create modern-language-association-for-JEI.csl (#6593) 6cb436997b Partial update of APA styles for 1.0.2, including software, legal, most localizations (#6270) d7c4ebec85 fix film/video authors for american-sociological-association.csl (#6602) a97dbb32c5 Update studii-teologice.csl (#6591) e19e08780e Update acm-sig-proceedings-long-author-list.csl (#6594) c8abbcdd88 Update acm-sig-proceedings.csl (#6595) 725ace4a40 Create uppsala-university-library-harvard.csl (#6574) a973041e0e update bluebook-law-review.csl (#6583) 0891cfc49a Update masarykova-univerzita-pravnicka-fakulta.csl (#6589) 80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581) b6fb00e415 Create arachnologische-mitteilungen.csl (#6375) 2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580) fd230a7073 Create veterinary-clinical-pathology.csl (#6372) ac0afa3cae Create dedicated Basque APA file (#6370) fee0677a88 Update chicago-author-date.csl (#6289) ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230) f4116db325 Create Turcica.csl (#6240) 0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211) e73bf46674 Update vancouver.csl (#6156) b73c3ef193 Create independent TF AIP Style befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181) 048c9bddbc Add csl for SDMI (#6129) 1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567) b77084255f Update the-quarterly-journal-of-economics.csl (#6572) cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577) 9e9e08c219 Update isara-iso-690.csl (#6578) 4b4e8f442d Create silva-fennica.csl (#6568) dd8760bb2b Update american-journal-of-botany.csl (#6569) 8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560) 016050c4b7 Update geographie-et-cultures.csl (#6563) e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 0749a19b8306f2e8dcb9bf1a2e3a6992666030ac

dimitra-karadima added 2 commits April 4, 2020 19:39

Unit tests improvement for JabRef#6207

b9c73fd

Add the following changes: -remove multiple assert statements in test cases -split each assert statement in different test methods with meaningful test names

Fix some problems

b37a921

Fix some encoding problems Fix merge conflict with the master branch

koppor reviewed Apr 5, 2020

View reviewed changes

dimitra-karadima added 2 commits April 10, 2020 23:55

Fix merge conflict

fe9546d

Add more unit tests

02e8f67

Add the following changes: -move tests from IntegrityCheckTest to the appropriate Test classes -remove multiple assert statements in different test methods -fix the capitalization in TitleCheckerTest

dimitra-karadima requested a review from koppor April 12, 2020 15:00

Solve some issues

b204b6b

Change two failing tests to succesfull Remove one test

Merge from upstream/master

0595351

tobiasdiez marked this pull request as draft April 17, 2020 16:07

koppor requested changes Apr 22, 2020

View reviewed changes

koppor changed the title ~~[WIP] Add simple Unit Tests for #6207~~ Add simple Unit Tests for #6207 Apr 23, 2020

dimitra-karadima requested a review from koppor April 23, 2020 23:28

LinusDietz added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 28, 2020

tobiasdiez marked this pull request as ready for review April 28, 2020 06:57

koppor approved these changes May 5, 2020

View reviewed changes

Merge branch 'master' into fix-for-issue-6207

c9320d4

dimitra-karadima and others added 3 commits May 5, 2020 22:41

Fix conflicts

ed2e7ea

Merge master to branch

dc72c05

Merge branch 'master' into fix-for-issue-6207

3933821

koppor self-requested a review May 6, 2020 22:59

koppor requested changes May 6, 2020

View reviewed changes

dimitra-karadima added 4 commits May 7, 2020 15:34

Rework IntegrityCheckTest and BibtexKeyCheckerTest

ca9f2f4

Add the following changes: -Revert all methods from protected static to private -Change BibtexKeyCheckerTest in order not to use IntegrityCheckTest's methods -Add two tests methods that were moved to another class

Change JournalInAbbreviationListCheckerTest

207b28f

Due to recent merges to master the JournalAbbreviationRepository constructor changed so the creation of the object changes as well.

Remove @beforeeach from test classes

1544c73

Remove it wherever it was just creating objects

Rework EditionCheckerTest

16129ca

Remove unnecessary BibEntry initialization and import

dimitra-karadima requested a review from koppor May 7, 2020 12:49

koppor requested changes May 8, 2020

View reviewed changes

koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 8, 2020

Fix checkstyle

870ce0d

Rework test class because checkstyle is failing

dimitra-karadima requested a review from koppor May 9, 2020 09:56

koppor requested changes May 11, 2020

View reviewed changes

dimitra-karadima added 2 commits May 12, 2020 00:14

Fix checkstyle again

8529050

Fix because instance variable definition was in wrong order.

Rework PersonNamesCheckersTest

193d420

Change from multiple test methods to two parameterized with @MethodSource

dimitra-karadima requested a review from koppor May 11, 2020 21:33

koppor mentioned this pull request May 12, 2020

Make pull request number a link in "Files changed" refined-github/refined-github#3082

Closed

koppor requested changes May 12, 2020

View reviewed changes

dimitra-karadima added 2 commits May 12, 2020 16:01

Update PersonNamesCheckerTest

08afd12

Change the return type of two methods from String[] to Stream<String> since it is more readable.

Update IntegrityCheckTest

4eae95a

Add the following changes: -Add Javadoc -Rewrite testAuthorNameChecks with @MethodSource

dimitra-karadima requested a review from koppor May 12, 2020 13:10

koppor changed the base branch from master to dimitra-karadima-fix-for-issue-6207 May 13, 2020 05:08

koppor merged commit fdef92e into JabRef:dimitra-karadima-fix-for-issue-6207 May 13, 2020

koppor approved these changes May 13, 2020

View reviewed changes

	void fieldAcceptsASCIICharacters() {
	void fieldAcceptsAsciiCharacters() {

	String[] data = new String[] {"", "Knuth", "Donald E. Knuth and Kurt Cobain and A. Einstein"};
	return Stream.of("", "Knuth", "Donald E. Knuth and Kurt Cobain and A. Einstein");

Add simple Unit Tests for #6207 #6240

Add simple Unit Tests for #6207 #6240

Conversation

dimitra-karadima commented Apr 4, 2020 • edited by calixtus Loading

Siedlerchr commented Apr 4, 2020 • edited Loading

Choose a reason for hiding this comment

calixtus commented Apr 5, 2020 • edited Loading

dimitra-karadima commented Apr 5, 2020

tobiasdiez commented Apr 10, 2020

dimitra-karadima commented Apr 10, 2020

dimitra-karadima commented Apr 14, 2020

koppor left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimitra-karadima Apr 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented May 5, 2020

dimitra-karadima commented May 5, 2020

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented May 8, 2020

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimitra-karadima May 9, 2020 • edited Loading

Choose a reason for hiding this comment

koppor commented May 11, 2020

Choose a reason for hiding this comment

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented May 13, 2020

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimitra-karadima commented Apr 4, 2020 •

edited by calixtus

Loading

Siedlerchr commented Apr 4, 2020 •

edited

Loading

calixtus commented Apr 5, 2020 •

edited

Loading

koppor left a comment •

edited

Loading

dimitra-karadima Apr 23, 2020 •

edited

Loading

dimitra-karadima May 9, 2020 •

edited

Loading