Vorteil / Nachteil der Deklaration aller Variablen in einem JUnit-Test

Ich habe einige Komponententests für neuen Code bei der Arbeit geschrieben und ihn zur Codeüberprüfung gesendet. Einer meiner Mitarbeiter machte einen Kommentar dazu, warum ich Variablen, die in einer Reihe dieser Tests verwendet werden, außerhalb des Testbereichs platzierte.

Der Code, den ich gepostet habe, war im Wesentlichen

import org.junit.Test;

public class FooUnitTests {

    @Test
    public void testConstructorWithValidName() {
        new Foo(VALID_NAME);
    }

    @Test(expected = IllegalArgumentException.class)
    public void testConstructorWithNullName() {
        new Foo(null);
    }

    @Test(expected = IllegalArgumentException.class)
    public void testConstructorWithZeroLengthName() {
        new Foo("");
    }

    @Test(expected = IllegalArgumentException.class)
    public void testConstructorWithLeadingAndTrailingWhitespaceInName() {
        final String name = " " + VALID_NAME + " ";
        final Foo foo = new Foo(name);

        assertThat(foo.getName(), is(equalTo(VALID_NAME)));
    }

    private static final String VALID_NAME = "name";
}

Seine vorgeschlagenen Änderungen waren im Wesentlichen

import org.junit.Test;

public class FooUnitTests {

    @Test
    public void testConstructorWithValidName() {
        final String name = "name";
        final Foo foo = new Foo(name);
    }

    @Test(expected = IllegalArgumentException.class)
    public void testConstructorWithNullName() {
        final String name = null;
        final Foo foo = new Foo(name);
    }

    @Test(expected = IllegalArgumentException.class)
    public void testConstructorWithZeroLengthName() {
        final String name = "";
        final Foo foo = new Foo(name);
    }

    @Test(expected = IllegalArgumentException.class)
    public void testConstructorWithLeadingAndTrailingWhitespaceInName() {
        final String name = " name ";
        final Foo foo = new Foo(name);

        final String actual = foo.getName();
        final String expected = "name";

        assertThat(actual, is(equalTo(expected)));
    }
}

Wo alles , was im Rahmen des Tests erforderlich ist, im Rahmen des Tests definiert wird.

Einige der Vorteile, die er argumentierte, waren

1. Jeder Test ist in sich geschlossen.
2. Jeder Test kann isoliert oder aggregiert mit demselben Ergebnis ausgeführt werden.
3. Der Prüfer muss nicht dorthin scrollen, wo diese Parameter deklariert sind, um den Wert zu ermitteln.

Einige der Nachteile seiner Methode, die ich argumentierte, waren

1. Erhöht die Codeduplizierung
2. Kann dem Prüfer Rauschen hinzufügen, wenn mehrere ähnliche Tests mit unterschiedlichen Werten definiert sind (dh Test mit doFoo(bar)Ergebnissen in einem Wert, während derselbe Aufruf ein anderes Ergebnis hat, da er barin dieser Methode unterschiedlich definiert ist).

Gibt es neben der Konvention noch andere Vor- und Nachteile bei der Verwendung einer der beiden Methoden gegenüber der anderen?

Sie sollten weitermachen, was Sie tun.

Sich zu wiederholen ist im Testcode genauso schlecht wie im Geschäftscode und aus dem gleichen Grund. Ihr Kollege wurde durch die Idee irregeführt, dass jeder Test in sich geschlossen sein sollte . Das ist ganz richtig, aber "in sich geschlossen" bedeutet nicht, dass es alles, was es braucht, in seinem Methodenkörper enthalten sollte. Dies bedeutet nur, dass das gleiche Ergebnis erzielt werden soll, unabhängig davon, ob es isoliert oder als Teil einer Suite ausgeführt wird, und zwar unabhängig von der Reihenfolge der Tests in der Suite. Mit anderen Worten, der Testcode sollte dieselbe Semantik haben, unabhängig davon, welcher andere Code zuvor ausgeführt wurde . Es muss nicht der gesamte erforderliche Code in Textform gebündelt sein .

Die Wiederverwendung von Konstanten, Setup-Code und dergleichen erhöht die Qualität des Testcodes und beeinträchtigt nicht dessen Eigenständigkeit. Daher ist es gut, dass Sie dies weiterhin tun.

Es hängt davon ab, ob. Im Allgemeinen ist es eine gute Sache, Wiederholungskonstanten nur an einer Stelle deklariert zu haben.

In Ihrem Beispiel macht es jedoch keinen Sinn, ein Mitglied VALID_NAMEauf die gezeigte Weise zu definieren. Angenommen, in der zweiten Variante ändert ein Betreuer den Text nameim ersten Test, der zweite Test ist höchstwahrscheinlich noch gültig und umgekehrt. Nehmen wir beispielsweise an, Sie möchten zusätzlich Groß- und Kleinbuchstaben testen, aber auch einen Testfall nur mit Kleinbuchstaben und so wenig Testfällen wie möglich behalten. Anstatt einen neuen Test hinzuzufügen, können Sie den ersten Test in ändern

public void testConstructorWithValidName() {
    final String name = "Name";
    final Foo foo = new Foo(name);
}

und behalten Sie immer noch die restlichen Tests.

Tatsächlich können viele Tests, die von einer solchen Variablen abhängen, und das Ändern des Werts von VALID_NAMEspäter einige Ihrer Tests zu einem späteren Zeitpunkt unbeabsichtigt unterbrechen . Und in einer solchen Situation können Sie in der Tat die Selbstbeherrschung Ihrer Tests verbessern, indem Sie keine künstliche Konstante mit unterschiedlichen Tests einführen.

Was @KilianFoth darüber schrieb, sich nicht zu wiederholen, ist jedoch auch richtig: Befolgen Sie das DRY-Prinzip im Testcode genauso wie im Geschäftscode. Führen Sie beispielsweise Konstanten oder Elementvariablen ein, wenn es für Ihren Testcode wichtig ist, dass ihr Wert an jeder Stelle gleich ist.

Ihr Beispiel zeigt, wie Tests dazu neigen, den Initialisierungscode zu wiederholen. Dies ist ein typischer Ausgangspunkt für das Refactoring. Sie können also in Betracht ziehen, die Wiederholung von zu überarbeiten

  new Foo(...);

in eine separate Funktion

 public Foo ConstructFoo(String name) 
 {
     return new Foo(name);
 }

da Sie so die Signatur des FooKonstruktors zu einem späteren Zeitpunkt einfacher ändern können (da es weniger Stellen gibt, an denen new Footatsächlich aufgerufen wird).

Ich würde eigentlich ... nicht für beides gehen , aber ich werde deins über deinen Kollegen für dein vereinfachtes Beispiel auswählen.

Das Wichtigste zuerst: Ich tendiere dazu, Inlining-Werte zu bevorzugen, anstatt einmalige Aufgaben zu erledigen. Dies verbessert die Lesbarkeit des Codes für mich, da ich meinen Gedankengang nicht in mehrere Zuweisungsanweisungen aufteilen und dann die Codezeile (n) lesen muss, in der sie verwendet werden. Daher werde ich Ihren Ansatz Ihrem Kollegen vorziehen, mit dem leichten Trottel, der testConstructorWithLeadingAndTrailingWhitespaceInName()wahrscheinlich nur sein kann:

assertThat(new Foo(" " + VALID_NAME + " ").getName(), is(equalTo(VALID_NAME)));

Das bringt mich zur Benennung. Wenn Ihr Test behauptet, dass ein Test Exceptionausgelöst werden soll, sollte Ihr Testname normalerweise auch dieses Verhalten zur Klarheit beschreiben. Was ist also, wenn der Name beim Erstellen meines FooObjekts zusätzliche Leerzeichen enthält ? Und wie hängt das mit dem Werfen zusammen IllegalArgumentException?

Aufgrund der nullund ""Tests gehe ich hier davon aus, dass Exceptiondiesmal dasselbe für den Anruf geworfen wird getName(). Wenn dies tatsächlich der Fall ist, ist es möglicherweise besser, den Namen der Testmethode auf callingGetNameWithWhitespacePaddingThrows()( IllegalArgumentExceptionwas meiner Meinung nach Teil der Ausgabe von JUnit ist und daher optional ist) festzulegen. Wenn das Auffüllen von Leerzeichen im Namen während der Instanziierung ebenfalls dasselbe Exceptionauslöst, überspringe ich die Behauptung ebenfalls vollständig, um zu verdeutlichen, dass dies im Verhalten des Konstruktors liegt.

Ich denke jedoch, dass es hier eine bessere Möglichkeit gibt, Dinge zu tun, wenn Sie mehr Permutationen für gültige und ungültige Eingaben erhalten, nämlich parametrisierte Tests . Was Sie testen, ist genau das: Sie erstellen eine Reihe von FooObjekten mit unterschiedlichen Konstruktorargumenten und behaupten dann, dass dies entweder bei der Instanziierung oder beim Aufruf fehlschlägt getName(). Lassen Sie JUnit daher die Iteration und das Gerüst für Sie ausführen. Sie können JUnit in Laienbegriffen mitteilen, "mit welchen Werten ich ein FooObjekt erstellen möchte ", und dann Ihre Testmethode bestätigen lassenIllegalArgumentExceptionan den richtigen Stellen. Da JUnits Methode des parametrisierten Testens nicht gut funktioniert, wenn sowohl erfolgreiche als auch außergewöhnliche Fälle im selben Test getestet werden (soweit mir bekannt ist), müssen Sie wahrscheinlich ein bisschen durch einen Reifen springen die folgende Struktur:

@RunWith(Parameterized.class)
public class FooUnitTests {

    private static final String VALID_NAME = "ABC";

    @Parameters
    public static Collection<Object[]> data() {
        return Arrays.asList(new Object[][] {
                 { VALID_NAME, false }, 
                 { "", true }, 
                 { null, true }, 
                 { " " + VALID_NAME + " ", true }
           });
    }

    private String input;

    private boolean throwException;

    public FooUnitTests(String input, boolean throwException) {
        this.input = input;
        this.throwException = throwException;
    }

    @Test
    public void test() {
        try {
            Foo test = new Foo(input);
            // TODO any testing required for VALID_NAME?
            if (throwException) {
                assertEquals(VALID_NAME, test.getName());
            }
        } catch (IllegalArgumentException e) {
            if (!throwException) {
                throw new RuntimeException(e);
            }
        }
    }
}

Zugegeben, dies sieht für einen ansonsten einfachen Test mit vier Werten umständlich aus, und die etwas restriktive Implementierung von JUnit hilft nicht viel. In dieser Hinsicht finde ich den@DataProvider Ansatz von TestNG nützlicher, und es lohnt sich auf jeden Fall, genauer hinzuschauen, wenn Sie parametrisierte Tests in Betracht ziehen.

Hier ist, wie man TestNG für Ihre Unit-Tests verwenden kann (mit Java 8 Stream, um die IteratorKonstruktion zu vereinfachen ). Einige der Vorteile sind, ohne darauf beschränkt zu sein ,:

1. Testparameter werden zu Methodenargumenten, nicht zu Klassenfeldern.
2. Sie können mehrere @DataProviders haben, die verschiedene Arten von Eingaben bereitstellen.
   • Es ist wohl klarer und einfacher, neue gültige / ungültige Eingaben zum Testen hinzuzufügen.
3. Für Ihr vereinfachtes Beispiel sieht es immer noch etwas übertrieben aus, aber meiner Meinung nach ist es ordentlicher als JUnits Ansatz.

public class FooUnitTests {

    private static final String VALID_NAME = "ABC";

    @DataProvider(name="successes")
    public static Iterator<Object[]> getSuccessCases() {
        return Stream.of(VALID_NAME).map(v -> new Object[]{ v }).iterator();
    }

    @DataProvider(name="exceptions")
    public static Iterator<Object[]> getExceptionCases() {
        return Stream.of("", null, " " + VALID_NAME + " ")
                    .map(v -> new Object[]{ v }).iterator();
    }

    @Test(dataProvider="successes")
    public void testSuccesses(String input) {
        new Foo(input);
        // TODO any further testing required?
    }

    @Test(dataProvider="exceptions", expectedExceptions=IllegalArgumentException.class)
    public void testExceptions(String input) {
        Foo test = new Foo(input);
        if (input.contains(VALID_NAME)) {
            // TestNG favors assert(actual, expected).
            assertEquals(test.getName(), VALID_NAME);
        }
    }
}