Skip to content
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

Add simple unit tests #7543

Merged
merged 9 commits into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package org.jabref.gui.autocompleter;

import org.jabref.logic.journals.JournalAbbreviationRepository;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;
import org.jabref.model.entry.field.SpecialField;
import org.jabref.model.entry.field.StandardField;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertSame;
import static org.mockito.Mockito.mock;

class SuggestionProvidersTest {

@Test
void getForFieldTest() {
BibDatabase database = new BibDatabase();
JournalAbbreviationRepository abbreviationRepository = mock(JournalAbbreviationRepository.class);
BibEntry entry = new BibEntry();
Field personEntryField = StandardField.AUTHOR;
Field singleEntryField = StandardField.XREF;
Field multipleEntryField = StandardField.XDATA;
Field journalEntryField = StandardField.JOURNAL;
Field publisherEntryField = StandardField.PUBLISHER;
Field specialEntryField = SpecialField.PRINTED;
AutoCompletePreferences autoCompletePreferences = new AutoCompletePreferences(true, AutoCompleteFirstNameMode.BOTH, AutoCompletePreferences.NameFormat.BOTH, FieldFactory.parseFieldList(personEntryField.getName() + ";" + singleEntryField.getName() + ";" + multipleEntryField.getName() + ";" + journalEntryField.getName() + ";" + publisherEntryField.getName() + ";" + specialEntryField.getName()), null);
SuggestionProviders sp = new SuggestionProviders(database, abbreviationRepository, autoCompletePreferences);
SuggestionProviders empty = new SuggestionProviders();

entry.setField(personEntryField, "Goethe");
entry.setField(singleEntryField, "Single");
entry.setField(multipleEntryField, "Multiple");
entry.setField(journalEntryField, "Journal");
entry.setField(publisherEntryField, "Publisher");
entry.setField(specialEntryField, "2000");
database.insertEntry(entry);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the database constructed here? Where is it used? Maybe, the whole part can be removed?


assertSame("org.jabref.gui.autocompleter.EmptySuggestionProvider", empty.getForField(personEntryField).getClass().getName());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls use assertEquals, because of consistency reasons to other tests (assertSame is seldmoly used).

Suggested change
assertSame("org.jabref.gui.autocompleter.EmptySuggestionProvider", empty.getForField(personEntryField).getClass().getName());
assertEquals(org.jabref.gui.autocompleter.EmptySuggestionProvider.class, empty.getForField(personEntryField).getClass());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(applies for all lines here)

assertSame("org.jabref.gui.autocompleter.PersonNameSuggestionProvider", sp.getForField(personEntryField).getClass().getName());
assertSame("org.jabref.gui.autocompleter.BibEntrySuggestionProvider", sp.getForField(singleEntryField).getClass().getName());
assertSame("org.jabref.gui.autocompleter.BibEntrySuggestionProvider", sp.getForField(multipleEntryField).getClass().getName());
assertSame("org.jabref.gui.autocompleter.JournalsSuggestionProvider", sp.getForField(journalEntryField).getClass().getName());
assertSame("org.jabref.gui.autocompleter.JournalsSuggestionProvider", sp.getForField(publisherEntryField).getClass().getName());
assertSame("org.jabref.gui.autocompleter.WordSuggestionProvider", sp.getForField(specialEntryField).getClass().getName());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,9 @@ void testLastPage() {
assertEquals("97", CitationKeyGenerator.lastPage("7,41,73--97"));
assertEquals("97", CitationKeyGenerator.lastPage("7,41,97--73"));
assertEquals("43", CitationKeyGenerator.lastPage("43+"));
assertEquals("0", CitationKeyGenerator.lastPage("00--0"));
assertEquals("1", CitationKeyGenerator.lastPage("1--1"));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please no empty line at the end of the method

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please convert the whole test to a @ParameterizedTest. See #7544 for an inspiration

}

@SuppressWarnings("ConstantConditions")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.jabref.logic.citationstyle;

import java.util.ArrayList;
import java.util.List;

import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertNotNull;

class CitationStyleCacheTest {

private BibEntry bibEntry;
private List<BibEntry> entries;
private BibDatabase database;
private BibDatabaseContext databaseContext;
private CitationStyleCache csCache;

@Test
void getCitationForTest() {
BibEntry bibEntry = new BibEntry();
bibEntry.setCitationKey("test");
List<BibEntry> entries = new ArrayList<>();
entries.add(0, bibEntry);
BibDatabase database = new BibDatabase(entries);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't one do the following?

Suggested change
BibEntry bibEntry = new BibEntry();
bibEntry.setCitationKey("test");
List<BibEntry> entries = new ArrayList<>();
entries.add(0, bibEntry);
BibDatabase database = new BibDatabase(entries);
BibDatabase database = new BibDatabase(List.of(new BibEntry("test"));

BibDatabaseContext databaseContext = new BibDatabaseContext(database);
CitationStyleCache csCache = new CitationStyleCache(databaseContext);

assertNotNull(csCache.getCitationFor(bibEntry));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please test for a concrete result.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ public void test() {
assertEquals("UPPER", formatter.format("UPPER"));
assertEquals("UPPER {lower}", formatter.format("upper {lower}"));
assertEquals("UPPER {l}OWER", formatter.format("upper {l}ower"));
assertEquals("1", formatter.format("1"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please convert to @ParameterizedTests

assertEquals("!", formatter.format("!"));

}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,22 @@
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class HTMLCharacterCheckerTest {

private final HTMLCharacterChecker checker = new HTMLCharacterChecker();
private final BibEntry entry = new BibEntry();

@Test
void fieldNullValueCheck() {
Exception exception = assertThrows(
NullPointerException.class,
() -> entry.setField(StandardField.AUTHOR, null),
"field value must not be null"
);
}

@Test
void titleAcceptsNonHTMLEncodedCharacters() {
entry.setField(StandardField.TITLE, "Not a single {HTML} character");
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/org/jabref/logic/journals/AbbreviationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

class AbbreviationTest {

Expand Down Expand Up @@ -101,4 +103,18 @@ void testDefaultAndShortestUniqueAbbreviationsAreSame() {
Abbreviation abbreviation = new Abbreviation("Long Name", "L N");
assertEquals(abbreviation.getAbbreviation(), abbreviation.getShortestUniqueAbbreviation());
}

@Test
void testToString() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JabRef does not rely on the toString contract in this case. Please remove the test.

If you want, you can check for nonNull of toString.

Abbreviation abbreviation = new Abbreviation("Long Name", "L N", "LN");
assertEquals(abbreviation.toString(), "Abbreviation{name=Long Name, abbreviation=L N, medlineAbbreviation=L N, shortestUniqueAbbreviation=LN}");
}

@Test
void testEquals() {
Abbreviation abbreviation = new Abbreviation("Long Name", "L N", "LN");
Abbreviation otherAbbreviation = new Abbreviation("Long Name", "L N", "LN");
assertTrue(abbreviation.equals(otherAbbreviation));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertTrue(abbreviation.equals(otherAbbreviation));
assertEquals(abbreviation, otherAbbreviation);

assertFalse(abbreviation.equals("String"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertFalse(abbreviation.equals("String"));
assertEquals("String", abbreviation);

}
}
63 changes: 63 additions & 0 deletions src/test/java/org/jabref/model/FieldChangeTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package org.jabref.model;

import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.StandardField;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

class FieldChangeTest {

private FieldChange Fieldchange;

@Test
void testHashCode() {
FieldChange fcNull = new FieldChange(null, null, null, null);
assertEquals(923521, fcNull.hashCode());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please no test for hashCode.

You can include https://jqno.nl/equalsverifier/ as dependency and work on that. Please do not test for that manually.


@Test
void testEquals() {
BibEntry entry = new BibEntry();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very hard to read test. Please use https://jqno.nl/equalsverifier/ or just delete the test.

I never used https://jqno.nl/equalsverifier/ for myself. Please add a proposal based on that and we can investigate whether this is really worth it or causes much trouble.

BibEntry entryOther = new BibEntry();
final Field field = StandardField.DOI;
final Field fieldOther = StandardField.DOI;
entry.setField(StandardField.DOI, "foo");
final String oldValue = "foo";
final String newValue = "bar";
final String oldValueOther = "fooX";
final String newValueOther = "barX";

FieldChange fc = new FieldChange(entry, field, oldValue, newValue);
FieldChange fcOther = new FieldChange(entryOther, fieldOther, oldValueOther, newValueOther);
FieldChange fcBlankAll = new FieldChange(null, null, null, null);
FieldChange fcBlankField = new FieldChange(entry, null, oldValue, newValue);
FieldChange fcBlankOldValue = new FieldChange(entry, field, null, newValue);
FieldChange fcBlankNewValue = new FieldChange(entry, field, oldValue, null);

assertFalse(fc.equals("foo"));
assertTrue(fc.equals(fc));
assertFalse(fcBlankAll.equals(fc));
assertFalse(fc.equals(fcOther));
assertFalse(fcBlankField.equals(fc));
assertFalse(fcBlankOldValue.equals(fc));
assertFalse(fcBlankNewValue.equals(fc));
assertTrue(fcBlankAll.equals(fcBlankAll));
}

@Test
void testToString() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though "Effective Java" recomments that toString is a contract; JabRef does not rely on that in this case. Please remove that test.

BibEntry entry = new BibEntry();
Field field = StandardField.DOI;
entry.setCitationKey("CitationKey");
final String oldValue = "Old";
final String newValue = "New";

FieldChange fc = new FieldChange(entry, field, oldValue, newValue);
assertEquals("FieldChange [entry=CitationKey, field=DOI, oldValue=Old, newValue=New]", fc.toString());
}
}
9 changes: 9 additions & 0 deletions src/test/java/org/jabref/model/entry/EntryLinkListTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class EntryLinkListTest {
Expand All @@ -21,6 +22,7 @@ public class EntryLinkListTest {
private ParsedEntryLink link;
private BibEntry source;
private BibEntry target;
private BibEntry entry;

@BeforeEach
public void before() {
Expand All @@ -29,6 +31,7 @@ public void before() {
link = links.get(0);
source = create("source");
target = create("target");
entry = create("entry");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not initalize the variable here. It is used only in one test. Just move the variable to givenBibEntryWhenParsingThenExpectLink.

}

private BibEntry create(String citeKey) {
Expand Down Expand Up @@ -59,6 +62,12 @@ public void givenFieldValueAndDatabaseWhenParsingThenExpectLink() {
assertEquals(expected, link);
}

@Test
public void givenBibEntryWhenParsingThenExpectLink() {
ParsedEntryLink expected = new ParsedEntryLink(entry);
assertFalse(expected.getLinkedEntry().isEmpty());
}

@Test
public void givenNullFieldValueAndDatabaseWhenParsingThenExpectLinksIsEmpty() {
links = EntryLinkList.parse(null, database);
Expand Down
7 changes: 7 additions & 0 deletions src/test/java/org/jabref/model/util/FileHelperTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jabref.model.util;

import java.nio.file.Path;
import java.util.Optional;

import org.junit.jupiter.api.Test;
Expand All @@ -18,4 +19,10 @@ public void fileExtensionFromUrl() {
final String filePath = "https://link.springer.com/content/pdf/10.1007%2Fs40955-018-0121-9.pdf";
assertEquals(Optional.of("pdf"), FileHelper.getFileExtension(filePath));
}

@Test
public void testFileNameEmpty() {
Path path = Path.of("/");
assertEquals(Optional.of(path), FileHelper.find("", path));
}
}