Skip to content

Commit

Permalink
Enable checkstyle for tests and fix style issues (#4378)
Browse files Browse the repository at this point in the history
* Enable checkstyle for tests

* Correct imports

* Reformat code

* Reformat code
  • Loading branch information
tobiasdiez authored Oct 17, 2018
1 parent 453e7b5 commit e7780ed
Show file tree
Hide file tree
Showing 251 changed files with 675 additions and 985 deletions.
8 changes: 2 additions & 6 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ dependencies {
testCompile 'org.reflections:reflections:0.9.11'
testCompile 'org.xmlunit:xmlunit-core:2.6.2'
testCompile 'org.xmlunit:xmlunit-matchers:2.6.2'
testCompile 'com.tngtech.archunit:archunit-junit:0.8.3'
testCompile 'com.tngtech.archunit:archunit-junit5-api:0.9.1'
testRuntime 'com.tngtech.archunit:archunit-junit5-engine:0.9.1'
testCompile "org.testfx:testfx-core:4.0.+"
testCompile "org.testfx:testfx-junit5:4.0.+"

Expand Down Expand Up @@ -402,12 +403,7 @@ jacocoTestReport {

// Code quality tasks
checkstyle {
// do not use other packages for checkstyle, excluding gen(erated) sources
checkstyleMain.source = "src/main/java"
toolVersion = '8.5'

// do not perform checkstyle checks by default
sourceSets = []
}

modernizer {
Expand Down
1 change: 0 additions & 1 deletion config/checkstyle/suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@

<suppressions>
<suppress checks="[a-zA-Z0-9]*" files="[\\/]gen[\\/]" />
<suppress checks="[a-zA-Z0-9]*" files="[\\/]test[\\/]" />
</suppressions>
4 changes: 2 additions & 2 deletions src/test/java/org/jabref/CatchExceptionsFromThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import org.junit.rules.ExternalResource;

/**
* JUnit by default ignores exceptions, which are reported via {@link Thread.UncaughtExceptionHandler}.
* With this rule also these kind of exceptions result in a failure of the test.
* JUnit by default ignores exceptions, which are reported via {@link Thread.UncaughtExceptionHandler}. With this rule
* also these kind of exceptions result in a failure of the test.
*/
public class CatchExceptionsFromThread extends ExternalResource {
@Override
Expand Down
1 change: 0 additions & 1 deletion src/test/java/org/jabref/JabRefPreferencesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,4 @@ void getDefaultEncodingReturnsPreviouslyStoredEncoding() {
prefs.setDefaultEncoding(StandardCharsets.UTF_8);
assertEquals(StandardCharsets.UTF_8, prefs.getDefaultEncoding());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ void firstPackageIsIndependentOfSecondPackage(String firstPackage, String second
Predicate<String> isExceptionPackage = (s) -> (s.startsWith("import " + secondPackage)
|| s.startsWith("import static " + secondPackage))
&& exceptions.getOrDefault(firstPackage, Collections.emptyList())
.stream()
.noneMatch(exception -> s.startsWith("import " + exception));
.stream()
.noneMatch(exception -> s.startsWith("import " + exception));

Predicate<String> isPackage = (s) -> s.startsWith("package " + firstPackage);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
package org.jabref.architecture;

import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.junit.AnalyzeClasses;
import com.tngtech.archunit.junit.ArchTest;
import com.tngtech.archunit.junit.ArchUnitRunner;
import com.tngtech.archunit.lang.ArchRule;
import org.junit.runner.RunWith;

import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;

@RunWith(ArchUnitRunner.class)
@AnalyzeClasses(packages = "org.jabref")
public class MainArchitectureTestsWithArchUnit {

@ArchTest
public static final ArchRule doNotUseApacheCommonsLang3 =
noClasses().that().areNotAnnotatedWith(ApacheCommonsLang3Allowed.class)
.should().accessClassesThat().resideInAPackage("org.apache.commons.lang3");

public static void doNotUseApacheCommonsLang3(JavaClasses classes) {
noClasses().that().areNotAnnotatedWith(ApacheCommonsLang3Allowed.class)
.should().accessClassesThat().resideInAPackage("org.apache.commons.lang3")
.check(classes);
}
}
39 changes: 19 additions & 20 deletions src/test/java/org/jabref/architecture/TestArchitectureTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,14 @@ public TestArchitectureTests() {
exceptions.add(CLASS_ORG_JABREF_UPDATE_TIMESTAMP_LISTENER_TEST);
exceptions.add(CLASS_ORG_JABREF_ENTRY_EDITOR_TEST);
exceptions.add(CLASS_ORG_JABREF_LINKED_FILE_VIEW_MODEL_TEST);

}

public static Stream<String[]> data() {
return Stream.of(
new String[][] {
{CLASS_ORG_JABREF_PREFERENCES},
{CLASS_ORG_JABREF_GLOBALS}
});
new String[][]{
{CLASS_ORG_JABREF_PREFERENCES},
{CLASS_ORG_JABREF_GLOBALS}
});
}

@ParameterizedTest
Expand All @@ -58,21 +57,21 @@ public void testsAreIndependent(String forbiddenPackage) throws IOException {

try (Stream<Path> pathStream = Files.walk(Paths.get("src/test/"))) {
List<Path> files = pathStream
.filter(p -> p.toString().endsWith(".java"))
.filter(p -> {
try {
return Files.readAllLines(p, StandardCharsets.UTF_8).stream().noneMatch(isExceptionClass);
} catch (IOException e) {
return false;
}
})
.filter(p -> {
try {
return Files.readAllLines(p, StandardCharsets.UTF_8).stream().anyMatch(isForbiddenPackage);
} catch (IOException e) {
return false;
}
}).collect(Collectors.toList());
.filter(p -> p.toString().endsWith(".java"))
.filter(p -> {
try {
return Files.readAllLines(p, StandardCharsets.UTF_8).stream().noneMatch(isExceptionClass);
} catch (IOException e) {
return false;
}
})
.filter(p -> {
try {
return Files.readAllLines(p, StandardCharsets.UTF_8).stream().anyMatch(isForbiddenPackage);
} catch (IOException e) {
return false;
}
}).collect(Collectors.toList());

assertEquals(Collections.emptyList(), files, "The following classes are not allowed to depend on " + forbiddenPackage);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ public void removedAtIndexOkay() {
ArgumentCaptor<ListDataEvent> argument = ArgumentCaptor.forClass(ListDataEvent.class);
verify(listener).intervalRemoved(argument.capture());
assertEquals(ListDataEvent.INTERVAL_REMOVED, argument.getValue().getType());

}

@Test
Expand All @@ -91,7 +90,5 @@ public void removedAtIndexgreaterListSizeDoesNothing() {
model.removeAtIndex((getDefaultFieldFormatterCleanups().size() + 1));

verifyZeroInteractions(listener);

}

}
1 change: 0 additions & 1 deletion src/test/java/org/jabref/cli/AuxCommandLineTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,4 @@ public void test() throws URISyntaxException, IOException {
assertEquals(2, newDB.getEntries().size());
}
}

}
6 changes: 3 additions & 3 deletions src/test/java/org/jabref/cli/JabRefCLITest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class JabRefCLITest {

@Test
void parsingLongOptions() {
JabRefCLI cli = new JabRefCLI(new String[] {"--nogui", "--import=some/file", "--output=some/export/file"});
JabRefCLI cli = new JabRefCLI(new String[]{"--nogui", "--import=some/file", "--output=some/export/file"});

assertEquals(Collections.emptyList(), cli.getLeftOver());
assertEquals("some/file", cli.getFileImport());
Expand All @@ -21,7 +21,7 @@ void parsingLongOptions() {

@Test
void parsingShortOptions() {
JabRefCLI cli = new JabRefCLI(new String[] {"-n", "-i=some/file", "-o=some/export/file"});
JabRefCLI cli = new JabRefCLI(new String[]{"-n", "-i=some/file", "-o=some/export/file"});

assertEquals(Collections.emptyList(), cli.getLeftOver());
assertEquals("some/file", cli.getFileImport());
Expand All @@ -31,7 +31,7 @@ void parsingShortOptions() {

@Test
void preferencesExport() {
JabRefCLI cli = new JabRefCLI(new String[] {"-n", "-x=some/file"});
JabRefCLI cli = new JabRefCLI(new String[]{"-n", "-x=some/file"});

assertEquals(Collections.emptyList(), cli.getLeftOver());
assertEquals("some/file", cli.getPreferencesExport());
Expand Down
1 change: 0 additions & 1 deletion src/test/java/org/jabref/gui/AWTExceptionHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,4 @@ public void assertNoExceptions() {
throw new AssertionError("Uncaught exception in EDT", list.get(0));
}
}

}
7 changes: 4 additions & 3 deletions src/test/java/org/jabref/gui/AbstractUITest.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ protected void onSetUp() {
}

/**
* Returns the absolute Path of the given relative Path
* The backlashes are replaced with forwardslashes b/c assertJ can't type the former one on windows
* Returns the absolute Path of the given relative Path The backlashes are replaced with forwardslashes b/c assertJ
* can't type the former one on windows
*
* @param relativePath the relative path to the resource database
*/
protected String getAbsolutePath(String relativePath) throws URISyntaxException {
Expand Down Expand Up @@ -94,7 +95,7 @@ protected void takeScreenshot(AbstractWindowFixture<?, ?, ?> dialog, String file
screenshotTaker.saveComponentAsPng(dialog.target(), file.toString());
}

protected void assertColumnValue(JTableFixture table, int rowIndex, int columnIndex, String selectionValue){
protected void assertColumnValue(JTableFixture table, int rowIndex, int columnIndex, String selectionValue) {
String[][] tableContent;
tableContent = table.contents();

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/jabref/gui/DialogTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
import javax.swing.JButton;
import javax.swing.JDialog;


import org.assertj.swing.core.GenericTypeMatcher;
import org.assertj.swing.dependency.jsr305.Nonnull;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

import static org.assertj.swing.finder.WindowFinder.findDialog;

/**
* This test has been split to work, the other part can be found at DialogTest2
*/
Expand Down
16 changes: 7 additions & 9 deletions src/test/java/org/jabref/gui/EntryTableTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,12 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;

/**
* Specific Use-Case:
* I import a database. Then I doubleclick on the first entry in the table to open the entry editor.
* Then I click on the first entry again, and scroll through all of the lists entries, without having to click
* on the table again.
* Specific Use-Case: I import a database. Then I doubleclick on the first entry in the table to open the entry editor.
* Then I click on the first entry again, and scroll through all of the lists entries, without having to click on the
* table again.
*/
@Tag("GUITest")
public class EntryTableTest extends AbstractUITest{
public class EntryTableTest extends AbstractUITest {

private final static int SCROLL_ACTION_EXECUTION = 5;
private final static String TEST_FILE_NAME = "testbib/testjabref.bib";
Expand Down Expand Up @@ -48,20 +47,19 @@ public void scrollThroughEntryList() throws Exception {
assertColumnValue(entryTable, 0, TITLE_COLUMN_INDEX, entryTable.selectionValue());

//go throught the table and check if the entry with the correct index is selected
for (int i=0; i < SCROLL_ACTION_EXECUTION; i++) {
for (int i = 0; i < SCROLL_ACTION_EXECUTION; i++) {
robot().pressAndReleaseKey(DOWN);
assertNotNull(entryTable.selectionValue());
assertColumnValue(entryTable, i+1, TITLE_COLUMN_INDEX, entryTable.selectionValue());
assertColumnValue(entryTable, i + 1, TITLE_COLUMN_INDEX, entryTable.selectionValue());
}
//do the same going up again
for (int i = SCROLL_ACTION_EXECUTION; i > 0; i--) {
robot().pressAndReleaseKey(UP);
assertNotNull(entryTable.selectionValue());
assertColumnValue(entryTable, i-1, TITLE_COLUMN_INDEX, entryTable.selectionValue());
assertColumnValue(entryTable, i - 1, TITLE_COLUMN_INDEX, entryTable.selectionValue());
}

closeDatabase();
exitJabRef();
}

}
2 changes: 0 additions & 2 deletions src/test/java/org/jabref/gui/IdFetcherDialogTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ public boolean test() {
entryTable.requireRowCount(1);
}


public static Stream<Object[]> instancesToTest() {
return Stream.of(
new Object[]{"BibTeX", "DOI", "10.1002/9781118257517"},
Expand All @@ -119,5 +118,4 @@ public static Stream<Object[]> instancesToTest() {
new Object[]{"biblatex", "ISBN", "9780321356680"}
);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -188,5 +188,4 @@ public static Stream<Object[]> instancesToTest() {
);
// @formatter:on
}

}
13 changes: 6 additions & 7 deletions src/test/java/org/jabref/gui/ParameterizedDialogTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ protected boolean isMatching(JDialog dialog) {
findDialog(matcher).withTimeout(10_000).using(robot()).close();
} else {
findDialog(matcher).withTimeout(10_000).using(robot())
.button(new GenericTypeMatcher<JButton>(JButton.class) {
.button(new GenericTypeMatcher<JButton>(JButton.class) {

@Override
protected boolean isMatching(@Nonnull JButton jButton) {
return buttonName.equals(jButton.getText());
}
}).click();
@Override
protected boolean isMatching(@Nonnull JButton jButton) {
return buttonName.equals(jButton.getText());
}
}).click();
}
if (createDatabase) {
closeDatabase();
Expand Down Expand Up @@ -130,5 +130,4 @@ public static Stream<Object[]> instancesToTest() {
);
// @formatter:on
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,4 @@ public static Stream<Object[]> instancesToTest() {
);
// @formatter:on
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

public class PersonNameSuggestionProviderTest {

private static final Author vassilisKostakos = new Author("Vassilis", "V.", "", "Kostakos", "");
private final Author vassilisKostakos = new Author("Vassilis", "V.", "", "Kostakos", "");
private PersonNameSuggestionProvider autoCompleter;
private BibEntry entry;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ public class FileAnnotationViewModelTest {
public void removeOnlyLineBreaksNotPrecededByPeriodOrColon() {
String content = "This is content";
String marking = String.format("This is paragraph 1.%n" +
"This is paragr-%naph 2, and it crosses%nseveral lines,%nnow you can see next paragraph:%n"
+ "This is paragraph%n3.");
"This is paragr-%naph 2, and it crosses%nseveral lines,%nnow you can see next paragraph:%n"
+ "This is paragraph%n3.");

FileAnnotation linkedFileAnnotation = new FileAnnotation("John", LocalDateTime.now(), 3, content, FileAnnotationType.FREETEXT, Optional.empty());
FileAnnotation annotation = new FileAnnotation("Jaroslav Kucha ˇr", LocalDateTime.parse("2017-07-20T10:11:30"), 1, marking, FileAnnotationType.HIGHLIGHT, Optional.of(linkedFileAnnotation));
Expand All @@ -30,8 +30,8 @@ public void removeOnlyLineBreaksNotPrecededByPeriodOrColon() {
assertEquals("This is content", annotationViewModel.getContent());

assertEquals(String.format("This is paragraph 1.%n" +
"This is paragraph 2, and it crosses several lines, now you can see next paragraph:%n"
+ "This is paragraph 3."),
annotationViewModel.getMarking());
"This is paragraph 2, and it crosses several lines, now you can see next paragraph:%n"
+ "This is paragraph 3."),
annotationViewModel.getMarking());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,4 @@ public void test() throws Exception {
List<LinkedFile> actual = util.findAssociatedNotLinkedFiles(entry);
assertEquals(expected, actual);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public void draggedOnTopOfGroupAddsBeforeItWhenSourceGroupWasBefore() throws Exc
@Test
public void entriesAreAddedCorrectly() {
String groupName = "group";
ExplicitGroup group = new ExplicitGroup(groupName, GroupHierarchyType.INDEPENDENT,',');
ExplicitGroup group = new ExplicitGroup(groupName, GroupHierarchyType.INDEPENDENT, ',');
BibEntry entry = new BibEntry();
databaseContext.getDatabase().insertEntry(entry);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void rootGroupIsAllEntriesByDefault() throws Exception {

@Test
public void explicitGroupsAreRemovedFromEntriesOnDelete() {
ExplicitGroup group = new ExplicitGroup("group", GroupHierarchyType.INDEPENDENT,',');
ExplicitGroup group = new ExplicitGroup("group", GroupHierarchyType.INDEPENDENT, ',');
BibEntry entry = new BibEntry();
databaseContext.getDatabase().insertEntry(entry);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ public class EntryFromFileCreatorManagerTest {
public void setUp() {
externalFileTypes = mock(ExternalFileTypes.class, Answers.RETURNS_DEEP_STUBS);
when(externalFileTypes.getExternalFileTypeByExt("pdf")).thenReturn(Optional.empty());

}

@Test
Expand All @@ -58,7 +57,7 @@ public void testGetCreator() {
@Test
public void testAddEntrysFromFiles() throws IOException {
try (FileInputStream stream = new FileInputStream(ImportDataTest.UNLINKED_FILES_TEST_BIB);
InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8)) {
InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8)) {
ParserResult result = new BibtexParser(prefs, new DummyFileUpdateMonitor()).parse(reader);
BibDatabase database = result.getDatabase();

Expand Down
Loading

0 comments on commit e7780ed

Please sign in to comment.