Spring JUnit Test: Code / Strukturierung / Beratung

[Kawu]

Hallo alle.

Ich bin relativ neu was das Schreiben von Tests angeht, also Vorsicht.

Beispielhaft gibt es folgende Methode:

public abstract class NamingUtil {

    public static final String NEGATIVE_NUMBER_MESSAGE = "Number is negative!";

    public static String convertToEnglishOrdinalStringFor(int number) {
        if (number < 0) {
            throw new IllegalArgumentException(NEGATIVE_NUMBER_MESSAGE);
        }

        String[] suffixes = new String[] {"th", "st", "nd", "rd", "th", "th", "th", "th", "th", "th"};

        switch (number % 100) {
            case 11:
            case 12:
            case 13:
                return number + "th";
            default:
                return number + suffixes[number % 10];
        }
    }
}

Die Methode konvertiert ganze Zahlen 0, 1, 2, 3, 4, ... usw. in englische Nummernstrings 0th, 1st, 2nd, 3rd, 4th, ...

Der Test sieht so aus:

@SpringBootTest
class ApplicationUnitTests {

    @Nested
    class NamingUtilTests {

        @Test
        @DisplayName("Test English number strings 1st, 2nd, 3rd, 4th, 100th, 101st, 102nd, 103rd, 104th, ...")
public void givenWholeNumber_whenConvertingToEnglishNumberString_thenVerifyCorrectness() {

            // given
            int zero = 0;
            int one = 1;
            int two = 2;
            int three = 3;
            int four = 4;
            int eleven = 11;
            int twelve = 12;
            int thirteen = 13;
            int fourteen = 14;

            int oneHundred = 100;
            int oneHundredOne = 101;
            int oneHundredTwo = 102;
            int oneHundredThree = 103;
            int oneHundredFour = 104;
            int oneHundredEleven = 111;
            int oneHundredTwelve = 112;
            int oneHundredThirteen = 113;
            int oneHundredFourteen = 114;

            // when
            String zeroth = NamingUtil.convertToEnglishOrdinalStringFor(zero);
            String first = NamingUtil.convertToEnglishOrdinalStringFor(one);
            String second = NamingUtil.convertToEnglishOrdinalStringFor(two);
            String third = NamingUtil.convertToEnglishOrdinalStringFor(three);
            String fourth = NamingUtil.convertToEnglishOrdinalStringFor(four);
            String eleventh = NamingUtil.convertToEnglishOrdinalStringFor(eleven);
            String twelfth = NamingUtil.convertToEnglishOrdinalStringFor(twelve);
            String thirteenth = NamingUtil.convertToEnglishOrdinalStringFor(thirteen);
            String fourteenth = NamingUtil.convertToEnglishOrdinalStringFor(fourteen);

            String oneHundredth = NamingUtil.convertToEnglishOrdinalStringFor(oneHundred);
            String oneHundredFirst = NamingUtil.convertToEnglishOrdinalStringFor(oneHundredOne);
            String oneHundredSecond = NamingUtil.convertToEnglishOrdinalStringFor(oneHundredTwo);
            String oneHundredThird = NamingUtil.convertToEnglishOrdinalStringFor(oneHundredThree);
            String oneHundredFourth = NamingUtil.convertToEnglishOrdinalStringFor(oneHundredFour);
            String oneHundredEleventh = NamingUtil.convertToEnglishOrdinalStringFor(oneHundredEleven);
            String oneHundredTwelfth = NamingUtil.convertToEnglishOrdinalStringFor(oneHundredTwelve);
            String oneHundredThirteenth = NamingUtil.convertToEnglishOrdinalStringFor(oneHundredThirteen);
            String oneHundredFourteenth = NamingUtil.convertToEnglishOrdinalStringFor(oneHundredFourteen);

            // then
            assertEquals("0th", zeroth);
            assertEquals("1st", first);
            assertEquals("2nd", second);
            assertEquals("3rd", third);
            assertEquals("4th", fourth);
            assertEquals("11th", eleventh);
            assertEquals("12th", twelfth);
            assertEquals("13th", thirteenth);
            assertEquals("14th", fourteenth);

            assertEquals("100th", oneHundredth);
            assertEquals("101st", oneHundredFirst);
            assertEquals("102nd", oneHundredSecond);
            assertEquals("103rd", oneHundredThird);
            assertEquals("104th", oneHundredFourth);
            assertEquals("111th", oneHundredEleventh);
            assertEquals("112th", oneHundredTwelfth);
            assertEquals("113th", oneHundredThirteenth);
            assertEquals("114th", oneHundredFourteenth);
        }

        @Test
        @DisplayName("Test English number strings for negative numbers")
public void givenNegativeNumber_whenConvertingToEnglishNumberString_thenThrowException() {

            // given
            int minusOne = -1;

            // when
            Exception minusOneException = assertThrows(IllegalArgumentException.class, () -> NamingUtil.convertToEnglishOrdinalStringFor(minusOne));

            // then
            assertEquals(NamingUtil.NEGATIVE_NUMBER_MESSAGE, minusOneException.getMessage()); // https://stackoverflow.com/a/46514550/396732
        }
    }

    @Nested
    class GameUtilTests {

    }
}

Wie man sieht enthält die Methode auch den -1 Exception-Fall.

Nun meine Fragen:

1. Ist der Methodenname givenNegativeNumber_whenConvertingToEnglishNumberString_thenThrowException OK (enterprise-würdig)?
2. Schreibt man Tests grundsätzlich so runter, d.h. die vielen Zeilen stumpfsinnigen Code? Enthält ja dann recht viel Redundanz... oder macht man das eher anders?
3. Sollte man den Fall für negative ganze Zahlen lieber in eine eigene Methode packen, da sich die Logik von den nicht-negativen ganzen Zahlen deutlich abhebt?:

@Test@DisplayName("Test English number strings for negative numbers")
public void givenNegativeNumber_whenConvertingToEnglishNumberString_thenThrowException() {

    // given
    int minusOne = -1;

    // when
    Exception minusOneException = assertThrows(IllegalArgumentException.class, () -> NamingUtil.convertToEnglishOrdinalStringFor(minusOne));

    // then
    assertEquals(NamingUtil.NEGATIVE_NUMBER_MESSAGE, minusOneException.getMessage());
}

Danke schonmal für die Hilfe

 

[Oneixee5]

Im ersten Moment würde ich sagen, du kannst eine Map oder eine Properties-Datei verwenden um alle Wert-Paare nacheinander durchzuprüfen. Das sollte sehr einfach umsetzbar sein, auch für die Exceptions.

 

[khmarbaise]

1. Frage wäre warum die Klasse NamingUtil abstrakt ist?
2. Warum ist ApplicationUnitTests mit @SpringBootTest annotiert?
Da Du einen Unit Test schreiben möchtest.. dann ist es besser eine Test Klasse NamingUtilTest.java zu erstellen ..

class NamingUtilTest {
   
    @Test
    void zerothToEnglish() {
        String zeroth = NamingUtil.convertToEnglishOrdinalStringFor(zero);
        assertEquals("0th", zeroth);
    }
   ..

}

Das dann für jede möglich Kombination... Ja jetzt wirst Du sagen, das sind viele Tests.. vollkommen richtig..
Dazu würde man eher einen parametrisierten Test schreiben..

class NamingUtilTest {
static Stream<Arguments> parameterized() {
return Stream.of(
  of(0, "0th"),
  of(1, "1st"),
  of(2, "2nd"),
  of(3, "3rd"),
  of(4, "4th"));
  }

  @ParameterizedTest
  @MethodSource
  void parameterized(int value, String expectedValue) {
    assertEquals(expectedValue, NamingUtil.convertToEnglishOrdinalStringFor(value));
  }

}
}

Eine entsprechende Abhängigkeit muss noch in der pom.xml angegeben sein (Vorausgesetzt Maven wird verwendet):

<dependency>
  <groupId>org.junit.jupiter</groupId>
  <artifactId>junit-jupiter-params</artifactId>
  <scope>test</scope>
</dependency>

Ja den Bereich der Fehler Prüfungen würde ich auf jeden Fall in einen anderen Test packen.
Da würde ich immer AssertJ, den JUnit Jupiter Assertions vorziehen und den Test wie folgt formulieren:

@Test
@DisplayName("Negative number should throw an exception")
void negativeNumberShouldThrowAnException() {
  assertThatIllegalArgumentException().isThrownBy(() -> NamingUtil.convertToEnglishOrdinalStringFor(-1))
    .withMessage("Number is negative!");
}

Wichtig: Ich würde niemals eine Konstante o.ä. aus dem produktiven Code referenzieren (hier für den Vergleich!). Zum einen weil der Produktiv Code dann diese dann zugreifbar mache muss (die Konstante NEGATIVE_NUMBER_MESSAGE muss public oder zumindest package private gemacht werden und das nur, um Testen zu können) und zum anderen weil dann eine Änderung am Code u.U. dazu führt dass dein Test trotzdem noch läuft, obwohl Du eine Änderung gemacht hast..(Die Verwendung der Konstante ist ein implementierungsdetail, dass man nicht nach außen transportieren bzw. Sichtbar machen sollte).

Testnamen sollten kurz sein, wenn möglich und einige dich etweder CamelCase Snake-Case aber nicht beides. ... oder wenn unbedingt Notwendig, dass per @DisplayName annotieren..

 

[Kawu]

Danke für die sehr hilfreiche Hilfe.

Die NamingUtil ist abstrakt, weil sie nicht instanziiert werden sollte, da bislang nur static-Methoden drin. Was ist der Hintergrund Deiner Frage? 🤔

Viele Leute propagieren diese givenAbc_whenDef_thenXyz() Konvention. Dem gegenüber steht die should... Variante. Das überfordert mich. Man muss sich hier wahrscheinlich für das eine oder das andere entscheiden.

Das mit der @SpringBootTest war nicht genau verstanden. Ist wohl eher für Integrationstests. Ich boote eine H2 DB per SQL scripts, mit der ich eigentlich Repos und REST services testen möchte, allerdings nicht mal sagen kann, dass ich von JUnit viel Ahnung hätte.

Da ist wohl zu lange bei mir was in immer der gleichen Spur hängen geblieben. Legacy JSF-Projekte... kein Spring. So gut wie keine Anforderungen an Testing. Entsprechendes Wissen.

Thx

 

[Manul]

Die NamingUtil ist abstrakt, weil sie nicht instanziiert werden sollte, da bislang nur static-Methoden drin.

In dem Fall würde sich eine finale Klasse mit privatem Konstruktor anbieten. Eine abstrakte Klasse hingegen ist das falsche Konzept, denn sie ist ja genau dafür da, dass von ihr abgeleitet (und sie damit indirekt instanziiert) werden kann. Das wäre also das Gegenteil von dem was du eigentlich erreichen willst.

 

[Manul]

Viele Leute propagieren diese givenAbc_whenDef_thenXyz() Konvention. Dem gegenüber steht die should... Variante. Das überfordert mich. Man muss sich hier wahrscheinlich für das eine oder das andere entscheiden.

Sagt wer? Wenn du niemanden (Team/Firma) im Rücken hast, der dir eine bestimmte Konvention zwingend für alles vorschreibt, kannst du auch erst mal deine eigenen Erfahrungen sammeln, und auch mal mit verschiedenen Ansätzen und Libraries für verschiedene Testarten rumspielen. Es gibt da nicht den einen goldenen Weg für alles. Gute Tests zu schreiben und vorallem zu wissen wann man was wie macht braucht Übung und Erfahrung.

Zumal ich das Given-When-Then-Pattern in solchen Fällen auch für ziemlich übertrieben und eher unpassend halte. Es handelt sich um einen recht einfachen Unittest für eine zustandslose statische Methode mit genau jeweils einem Eingabe- und Rückgabewert. Also nichts mit einem inneren oder äußeren Zustand oder Kontext welcher über ein "Given" zu definieren wäre, und auch das "When" und "Then" ist, wenn man es denn unbedingt in dieses Pattern quetschen will, nur der Methodenaufruf und eine Assertion auf dem Rückgabewert. Dafür reicht locker ein Einzeiler z.B.

assertThat(NamingUtil.convertToEnglishOrdinalStringFor(input)).isEqualTo(expected);

 

[M.L.]

JUnit

--> https://www.vogella.com/tutorials/JUnit/article.html (und man darf auch einzelne Spring-Bibliotheken und -Funktionalitäten inkludieren ohne das -Framework als Ganzes nutzen zu müssen)

 

[khmarbaise]

Die NamingUtil ist abstrakt, weil sie nicht instanziiert werden sollte, da bislang nur static-Methoden drin. Was ist der Hintergrund Deiner Frage? 🤔

Wie schon vorher erwähnt wurde, ist eine Abstrakte Klasse genau das Gegenteil (https://docs.oracle.com/javase/tutorial/java/IandI/abstract.html) . Eine Utils Klasse einfach final machen und einen privaten Konstruktor...

Viele Leute propagieren diese givenAbc_whenDef_thenXyz() Konvention. Dem gegenüber steht die should... Variante. Das überfordert mich. Man muss sich hier wahrscheinlich für das eine oder das andere entscheiden.

Im Methodennamen auf keinen Fall, da Du ja sonst das was in der Methode steht wiederholst... besser hier mit dem "shouldXXX" ansetzen.. braucht etwas Übang ist vollkommen klar...
Den given/when/then oder Arrange/Act/Assert sind Vorgehensweisen die aus dem BDD kommen (https://martinfowler.com/bliki/GivenWhenThen.html) wobei faktisch alle Unit Tests keine BDD sind.. somit macht das für mich auch keinen Sinn dort dem GWT Ansatz zu folgen...

Ich würde immer einen einfachen Unit Test vorziehen, wie vorher auch schon geschrieben wurde.

Das mit der @SpringBootTest war nicht genau verstanden. Ist wohl eher für Integrationstests. Ich boote eine H2 DB per SQL scripts, mit der ich eigentlich Repos und REST services testen möchte, allerdings nicht mal sagen kann, dass ich von JUnit viel Ahnung hätte.

Exakt. @SpringBootTest fährt nämlich den gesamten Context der Anwendung hoch, der in dem Fall völlig überflüssig ist..

Bei den REST Services ist die Frage, was überhaupt getestet werden soll? Dafür brauch man überhaupt keine H2
etc. Die Andere Frage wäre bei den Repos? Separat Testen? Trennst Du zwischen Repos/Services/Zugriffsschichten usw. ? Nutzt Du DTO's?

Gruß
Karl Heinz Marbaise

 

[Kawu]

In dem Fall würde sich eine finale Klasse mit privatem Konstruktor anbieten. Eine abstrakte Klasse hingegen ist das falsche Konzept, denn sie ist ja genau dafür da, dass von ihr abgeleitet (und sie damit indirekt instanziiert) werden kann. Das wäre also das Gegenteil von dem was du eigentlich erreichen willst.

Ach herrje, das kommt davon wenn man uralten Code herumkopiert und sich dazu keine Gedanken mehr macht. Leicht peinlich.

 

[Kawu]

...

Exakt. @SpringBootTest fährt nämlich den gesamten Context der Anwendung hoch, der in dem Fall völlig überflüssig ist..

Bei den REST Services ist die Frage, was überhaupt getestet werden soll? Dafür brauch man überhaupt keine H2
etc. Die Andere Frage wäre bei den Repos? Separat Testen? Trennst Du zwischen Repos/Services/Zugriffsschichten usw. ? Nutzt Du DTO's?

Gruß
Karl Heinz Marbaise

Es gibt eine Trennung zwischen Services/Repos und den REST-Ressourcen/Endpunkten.

In der DB sind knapp 50 Tabellen, die allemsamt mit mit JPA auf Entities gemappt sind. Entsprechend gibt es für die zu speichernden Entities jeweils ein Service/Repo. DTOs werden nicht verwendet, stattdessen ein Baum/Graph von Entities hin und zurück.

DTOs werden von mir nur deswegen verwendet, weil ich mir für die öffentlichen Seiten (sehr tabellarisch) die Daten aus vielen Tabellenspalten verschiedenster Tabellen per Queries raussuche und es auf diesen Seiten keine Speicherfunktionen gibt (nur Anzeige von Statisktiken - teilweise errechnet).

Nun, was soll getestet werden? Das ist eigentlich so die Preisfrage. Es handelt sich bei dem besagten Projekt um ein Hobbyprojekt, da ist also niemand der irgendwelche Vorgaben macht.