diff --git a/docs/adr/0009-use-plain-junit5-for-testing.md b/docs/adr/0009-use-plain-junit5-for-testing.md new file mode 100644 index 00000000000..35d58bf1097 --- /dev/null +++ b/docs/adr/0009-use-plain-junit5-for-testing.md @@ -0,0 +1,83 @@ +# Use Plain JUnit5 for advanced test assertions + +## Context and Problem Statement + +How to write readable test assertions? +How to write readable test assertions for advanced tests? + +## Considered Options + +* Plain JUnit5 +* Hamcrest +* AssertJ + +## Decision Outcome + +Chosen option: "Plain JUnit5", because comes out best \(see below\). + +### Positive Consequences + +* Tests are more readable +* More easy to write tests +* More readable assertions + +### Negative Consequences + +* More complicated testing leads to more complicated assertions + +## Pros and Cons of the Options + +### Plain JUnit5 + +Homepage: +JabRef testing guidelines: + +Example: + +```java +String actual = markdownFormatter.format(source); +assertTrue(actual.contains("Markup
")); +assertTrue(actual.contains("
  • list item one
  • ")); +assertTrue(actual.contains("
  • list item 2
  • ")); +assertTrue(actual.contains("> rest")); +assertFalse(actual.contains("\n")); +``` + +* Good, because Junit5 is "common Java knowledge" +* Bad, because complex assertions tend to get hard to read +* Bad, because no fluent API + +### Hamcrest + +Homepage: + +* Good, because offers advanced matchers (such as `contains`) +* Bad, because not full fluent API +* Bad, because entry barrier is increased + +### AssertJ + +Homepage: + +Example: + +```java +assertThat(markdownFormatter.format(source)) + .contains("Markup
    ") + .contains("
  • list item one
  • ") + .contains("
  • list item 2
  • ") + .contains("> rest") + .doesNotContain("\n"); +``` + +* Good, because offers fluent assertions +* Good, because allows partial string testing to focus on important parts +* Good, because assertions are more readable +* Bad, because not commonly used +* Bad, because newcomers have to learn an additional language to express test cases +* Bad, because entry barrier is increased +* Bad, because expressions of test cases vary from unit test to unit test + +## Links + +* German comparison between Hamcrest and AssertJ: diff --git a/src/test/java/org/jabref/architecture/MainArchitectureTestsWithArchUnit.java b/src/test/java/org/jabref/architecture/MainArchitectureTestsWithArchUnit.java index 6237103e62e..a21e086e326 100644 --- a/src/test/java/org/jabref/architecture/MainArchitectureTestsWithArchUnit.java +++ b/src/test/java/org/jabref/architecture/MainArchitectureTestsWithArchUnit.java @@ -47,6 +47,11 @@ public static void doNotUseGlyphsDirectly(JavaClasses classes) { noClasses().that().resideOutsideOfPackage("org.jabref.gui.icon").should().accessClassesThat().resideInAnyPackage("de.jensd.fx.glyphs", "de.jensd.fx.glyphs.materialdesignicons").check(classes); } + @ArchTest + public static void doNotUseAssertJ(JavaClasses classes) { + noClasses().should().accessClassesThat().resideInAPackage("org.assertj..").check(classes); + } + // "Currently disabled as there is no alternative for the rest of classes who need awt" @ArchIgnore @ArchTest