TestNG und File

Mit Threading und echten Files auf der Platte? Das ist kein Unit-Test mehr. Aber macht ja nichts, Integrationstests sind auch nicht schlecht. Hauptsache, man hat überhaupt einen Test.

In Integrationstests zu mocken kann IME schnell ins Chaos führen,

Das seh ich genauso. Mit dem Mocken meinte ich schon echte Unit-Tests.

Wie ich oben ja geschrieben habe, hatte ich mich mit dem Threading falsch ausgedrückt. Die Anwendung an sich ist zwar nebenläufig, die Klasse mit der die Dateien erzeugt werden, wird jedoch nicht nebenläufig getestet. Was sich meines Erachtens nach ohne übermäßigen Aufwand nicht vermeiden lässt, sind eben die Dateisystemzugriffe.

[QUOTE=cmrudolph]@Majora
ich habe das im ersten Moment für einen Scherz gehalten. [/QUOTE]

http://chadfowler.com/blog/2013/06/23/immutable-deployments/

Wenn man bedenkt das man im Endeffekt z.B. ein komplettes Image an Amazon schickt und dieses dort in 10-facher Ausführung auf deren Infrastruktur läuft.

Oder man sein Programm auf verschiedenen Betriebssystemen testen möchte.

File verhält sich ja nicht überall gleich.

Das sind Punkte, bei denen es interessant wird auch automatisiert die Infrastruktur bereitstellen zu können.

Das schreiben in eine Datei ist immer einen Seiteneffekt bei einem Programm. Das kann man oft auch umgehen, indem man Methoden z.B. einen String zurückliefern läßt, den man dann an einer anderen Stelle erst in die Datei schreibt. Ein Unit-Test würde sich dann darauf konzentrieren, daß der String korrekt ist.

File ist mMn. immer Integrationstest.

Ok, dann kann ich diese Klasse wohl nicht unittesten :wink: Denn sie implementiert nur ein einmethodiges Interface:
public void processMessage(List<String> message);

Im Konstruktor wird noch sichergestellt, dass der übergebene Pfad beschreibbar ist bzw. das Verzeichnis wird ggf. noch erstellt.
Es dreht sich also alles um das Schreiben von Strings in eine Datei. Insgesamt sind es nur etwa 30 SLOCs.

Das ist erstmal kein Grund, warum man sie nicht unittesten können soll. Vielleicht kann man sie testbarer machen durch Refactoring. Der Vorschlag von majora, die Funktionalität aufzuspalten, ist nicht schlecht. Du hättest dann eine Klasse (oder „Unit“), die die Daten berechnet - etwa als String oder OutputStream - und eine andere, die diese als Datei abspeichert. Das könnte man dann gut getrennt testen.
Du kannst den Code ja mal auf das notwendigste abspecken und hier zeigen.

Ich hab die Klassen mal unverändert gepastet, weil ich glaube, dass man nicht mehr sonderlich viel kürzen kann. Außerdem lasse ich die Verzeichnisse jetzt im Tempdir erzeugen.

MessageProcessor.java
[spoiler]```import java.util.List;

public interface MessageProcessor {
void processMessage(List message);
}


FileSavingProcessor.java
[spoiler]```import org.joda.time.DateTime;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Path;
import java.util.List;

public class FileSavingProcessor implements MessageProcessor {
    private final File saveDirectory;
    private final String lineSeparator = System.getProperty("line.separator");

    public FileSavingProcessor(Path saveDirectoryPath) {
        saveDirectory = saveDirectoryPath.toFile();
        if (!saveDirectory.exists()) //noinspection ResultOfMethodCallIgnored
            saveDirectory.mkdirs();
        if (!saveDirectory.isDirectory() || !saveDirectory.canWrite()) {
            throw new IllegalArgumentException("saveDirectoryPath must point to an existing, writable directory");
        }
    }

    @Override
    public void processMessage(List<String> message) {
        DateTimeFormatter fmt = DateTimeFormat.forPattern("yyyy-MM-dd_HH-mm-ss");
        String fileName = fmt.print(DateTime.now()) + ".txt";

        File logFile = new File(saveDirectory, fileName);
        try {
            //noinspection ResultOfMethodCallIgnored
            logFile.createNewFile();

            FileWriter fileWriter = new FileWriter(logFile);
            for (String line : message)
                fileWriter.write(line + lineSeparator);

            fileWriter.close();
        } catch (IOException e) {
            System.err.println("Failed to create logfile. Data couldn't be saved.");
        }
    }
}```[/spoiler]

FileSavingProcessorTest.java
[spoiler]```import org.joda.time.DateTime;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.testng.PowerMockTestCase;
import org.testng.FileAssert;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;

import static org.testng.Assert.assertEquals;

// TODO: use IObjectFactory / @ObjectFactory instead of extending PowerMockTestCase --- PowerMockito seems to have a classloading issue

@PrepareForTest(DateTime.class)
public class FileSavingProcessorTest extends PowerMockTestCase {
    private Path testLogDir;

    @BeforeMethod
    public void setUp() throws IOException {
        testLogDir = Files.createTempDirectory("FSP");
    }

    @Test
    public void createsLogDirectoryIfDoesntExist() {
        new FileSavingProcessor(testLogDir);

        final File dir = testLogDir.toFile();
        FileAssert.assertDirectory(dir);
    }

    @Test(expectedExceptions = {IllegalArgumentException.class})
    public void openingFileAsDirShouldThrowIAE() throws IOException {
        new FileSavingProcessor(Files.createTempFile("FSP", "dummyFile"));
    }

    @Test(expectedExceptions = {NullPointerException.class})
    public void shouldThrowNPEOnNullMessage() {
        final FileSavingProcessor fsp = new FileSavingProcessor(testLogDir);
        fsp.processMessage(null);
    }

    @Test
    public void shouldCreateNewFileOnIncomingMessage() throws Exception {
        final FileSavingProcessor fsp = new FileSavingProcessor(testLogDir);
        final List<String> message = new ArrayList<String>(0);

        PowerMockito.spy(DateTime.class);
        PowerMockito.doReturn(new DateTime(2013, 2, 1, 12, 45, 30)).when(DateTime.class, "now");

        fsp.processMessage(message);

        File logFile = testLogDir.resolve("2013-02-01_12-45-30.txt").toFile();

        FileAssert.assertFile(logFile);
    }

    @Test
    public void writtenFileShouldContainMessage() throws Exception {
        final FileSavingProcessor fsp = new FileSavingProcessor(testLogDir);
        final List<String> message = new ArrayList<String>(3);
        message.add("Line 1");
        message.add("");
        message.add("This is line 3");

        PowerMockito.spy(DateTime.class);
        PowerMockito.doReturn(new DateTime(2013, 2, 1, 12, 46, 30)).when(DateTime.class, "now");

        fsp.processMessage(message);

        List<String> written = Files.readAllLines(testLogDir.resolve("2013-02-01_12-46-30.txt"), Charset.defaultCharset());
        assertEquals(message, written);
    }
}
```[/spoiler]

Vielleicht habt ihr ja auch noch Kommentare zur Testklasse an sich.

Ich hab der Einfachheit halber die FileSavingProcessor-Klasse noch ein bisschen gekürzt und kleinere Methoden eingeführt:

public class FileSavingProcessor {

    private final File saveDirectory;
    private final String lineSeparator = System.getProperty("line.separator");

    public FileSavingProcessor(File saveDirectoryPath) {
        saveDirectory = saveDirectoryPath;
    }

    public void processMessage(List<String> message) {
        String fileName = createFilename();

        try {
            FileWriter fileWriter = createFileWriter(fileName);
            for (String line : message) {
                fileWriter.write(line + lineSeparator);
            }
            fileWriter.close();
        }
        catch (IOException e) {
            System.err.println("Failed to create logfile. Data couldn't be saved.");
        }
    }

    private String createFilename() {
        return "EinfachEinString.txt";
    }

    private FileWriter createFileWriter(String fileName) throws IOException {
        File logFile = new File(fileName);
        logFile.createNewFile();

        FileWriter fileWriter = new FileWriter(logFile);
        return fileWriter;
    }
}

Ein echter Unittest mit weggemockten IO-Klassen könnte so aussehen:

import java.io.File;
import java.io.FileWriter;
import java.util.Arrays;
import java.util.List;

import mockit.Expectations;

import org.testng.annotations.Test;

public class TestTest {

    private static final String lineSeparator = System.getProperty("line.separator");

    @Test
    public void test() throws Exception {
        FileSavingProcessor f = new FileSavingProcessor(new File("testdir"));
        List<String> s = Arrays.asList("eins", "zwei", "drei");

        new Expectations() {
            File file;
            FileWriter writer;

            {
                new File("EinfachEinString.txt");
                file.createNewFile();
                writer.write("eins" + lineSeparator);
                writer.write("zwei" + lineSeparator);
                writer.write("drei" + lineSeparator);
                writer.close();
            }
        };
        f.processMessage(s);
    }
}

Das ist natürlich 100% White-Box. Und weil so zentrale Klassen wie File und FileWriter gemockt werden, kommt man schnell in Schwierigkeiten, wenn diese indirekt durch aufgerufenen Code verwendet werden. (Dann müsste man NonStrictExceptations verwenden. Geht zwar, ist aber noch unschöner.) Um dieses Problem hier zu umgehen, habe ich den Dateinamen in einen einfachen String geändert.

Es geht also auch als Unittest. Trotzdem kann dein Integrationstest auch so bleiben. Ein Problem könnte dann aber sein, dass die Tests zu langsam werden, wenn man zuviel davon hat.

Die zwei kleinen Hilfsmethoden sind auf jeden Fall sinnvoll. Und im Konstruktor direkt ein File-Objekt zu übernehmen ist auch sehr sinnvoll (der Path kommt daher, dass ich mit nem String als Parameter angefangen habe und dann Stück für Stück refakturiert, aber offensichtlich noch nicht am Ende angekommen bin).

Wie ich dieses Whitebox-Testing finden soll, weiß ich noch nicht so recht… Auf jeden Fall koppelt man den Test dadurch extrem an die Implementierung anstatt ihn an das Verhalten zu koppeln.

Das ist so nicht richtig.

Alle echten Unittests sind Whitebox tests, genau darum geht es ja beim Unit Tests :wink:
D.h. diese sind immer an die Implementierung gekoppelt.
Was du (fälschlicherweise) mit „Verhalten“ meinst ist eher Blackbox Testing, dafür sind Integrationstests da, zB. kennst du die Schnittstelle, übergibst deine Daten und prüfst das Ergebnis, hast u.U. keine Ahung wie die Implementierung aussieht…

„Verghalten“ testen macht man zB. mit Mocks, die prüfen ob bestimmte Methoden aufgerufen wurden (u.U. mit einer festgelegten Reihenfolge). „Zustände“ testet man mit Mocks, die prüfen ob zB. Methoden mit entsprecchenden Werten aufgerufen wurden. "State vs. Behaviour testing " sind gute Suchbegriffe, auch hier geht es um Unittests.
Blackboxtests, also (meistens) Integrationstests, testen immer auch den Zustand, weil sie ja nicht ins System eingreifen sondern danach zB. Prüfen ob bestimmte Werte stimmen (in DBs, Dateien, etc. pp.).

Hier noch ein Link zu einem Fowler Artikel: Mocks Aren't Stubs

Nebenbei gesagt. java.io.File zu mocken ist ein sog. „Pain in the Ass“ in Java…

Das widerspricht so ziemlich allem, was ich in letzter Zeit über Testgetriebene Entwicklung gelesen habe. Dort findet sich auch der Begriff Greybox-Testing wieder.
Die Tests werden bereits vor der Implementierung geschrieben, können also gar nicht an die Implementierung gekoppelt sein. Stattdessen sollen sie das Verhalten einer Klasse spezifizieren. Wie das dann in der Klasse umgesetzt wird, steht dann ja auf einem ganz anderen Blatt. Und wenn es ein Whitebox-Test ist, dann kann ich mit meinem Test nicht mehr refakturieren - damit geht mir ein entscheidender Vorteil der testgetriebenen Entwicklung verloren.

Edit: Fowler deutet den Wert von Tests in “Refactoring” schon an, ansonsten beziehe ich mich auf “Growing Object-Oriented Software, Guided by Tests” von Freeman und Pryce sowie “Practical Unit Testing with TestNG and Mockito” von Kaczanowski.

Das widerspricht so ziemlich allem, was ich in letzter Zeit über Testgetriebene Entwicklung gelesen habe.

Vielleicht lesen wir nur unterschiedliche Dinge raus :wink:

Dort findet sich auch der Begriff Greybox-Testing wieder.

Ja, ist aber wie bereits gesagt gefährlicher (Chaos etc.).

Die Tests werden bereits vor der Implementierung geschrieben, können also gar nicht an die Implementierung gekoppelt sein.

Bei TDD geht man sogar in „Babyschritten“ vor, keine Zeile Prodcode ohne Test, nur soviel Prodcode wie nötig um den Test zu erfüllen.
Klar sind die Tests aber trotzdem an die Implementierung gekoppelt… sobald ein Test von den interna abhägt, ist er eben stark an die Implementierung gekoppelt, genau das macht man mit Mocks/Spies/Stubs → Whiteboxing eben, Unittests halt :slight_smile:

Stattdessen sollen sie das Verhalten einer Klasse spezifizieren. Wie das dann in der Klasse umgesetzt wird, steht dann ja auf einem ganz anderen Blatt.

Eben, mit Mocks/Spies/Stubs/Fakes :wink:

Und wenn es ein Whitebox-Test ist, dann kann ich mit meinem Test nicht mehr refakturieren - damit geht mir ein entscheidender Vorteil der testgetriebenen Entwicklung verloren.

Hier liegst du ganz falsch, speziell beim TDD -also Whitebox-Tests - ist refactoren einer der Kernbestandteile.
Das gehört zusammen, da widerspricht sich nix :wink:

Ich verstehe aber wie du darauf kommst…
Wenn man jetzt so ein bisschen drüber nachdenkt, merkt man dass man sich durch unsaubere Unittests und Prod. Code (speziell Redundanzen/„Überlappungen“) sich einen Riesenklotz ans Bein bindet der einen schnell „runterzieht“…
Stell dir vor du änderst eine Zeile im Prod Code, und musst danach 15 Stellen in den Tests ändern, oder umgekehrt.
Da hilft nur sehr „sauber“ sein, ständiges refactoren, vor auch in den Unittests.

Als Literaturempfehlung kann ich dir zu „XUnit: Refactoring Test Code“ raten.

Ach ja, soviel würde ich über TDD gar nicht lesen, der eigentliche Kampf findet gegen deine eigenen Gewohnheiten statt, nämlich deine bisherige Art Code zu schreiben :wink:

Red, Green, Refactor…

@maki : Ich bin heute dazu gekommen, das Essay von Fowler zu lesen. Insbesondere der Abschnitt Coupling Tests to Implementations spiegelt genau meine Bedenken bzgl. der Kopplung zwischen Implementierung und Test wider. Obwohl ich in den meisten anderen Bereichen wohl eher den „Mockist“-Weg vorziehe, hatte ich diesbezüglich wohl eher die „klassische“ Denkweise. Das zustandsbasierte Testing passt aber nicht so ganz zum verhaltensorientierten Testing beim mocken.

[QUOTE=maki]Hier liegst du ganz falsch, speziell beim TDD -also Whitebox-Tests - ist refactoren einer der Kernbestandteile.
Das gehört zusammen, da widerspricht sich nix ;)[/QUOTE]
Dass das zusammengehört, war mir schon bewusst. Ich habe nur nicht erkannt, dass der verhaltensbasierte Ansatz sich mit dem nicht widerspricht :slight_smile:

[QUOTE=maki;25073]Stell dir vor du änderst eine Zeile im Prod Code, und musst danach 15 Stellen in den Tests ändern, oder umgekehrt.
Da hilft nur sehr „sauber“ sein, ständiges refactoren, vor auch in den Unittests.[/QUOTE]
Genau das waren unter anderem meine Bedenken. Die Bedenken sind bei Einhaltung des DRY-Prinzips natürlich auch Grundlos - jedes Feature sollte ja nur genau einmal getestet werden wodurch derselbe Code nicht allzu oft durchlaufen wird.

Ich habe es mir mal auf die Amazon-Wunschliste gesetzt. Vielleicht komme ich in halbwegs naher Zukunft dazu, mich da durchzuarbeiten.

[QUOTE=maki;25073]Ach ja, soviel würde ich über TDD gar nicht lesen, der eigentliche Kampf findet gegen deine eigenen Gewohnheiten statt, nämlich deine bisherige Art Code zu schreiben :wink:

Red, Green, Refactor…[/QUOTE]
Genau dieses Mantra versuche ich mir einzubläuen (darauf wird ja auch überall hingewiesen, wenn man was zum Thema TDD liest). Aber es ist doch vorerst so viel schneller, „mal eben schnell“ dieses oder jenes Feature ohne den Test vorher geschrieben zu haben zu implementieren :wink:

@maki : Ich bin heute dazu gekommen, das Essay von Fowler zu lesen. Insbesondere der Abschnitt Coupling Tests to Implementations spiegelt genau meine Bedenken bzgl. der Kopplung zwischen Implementierung und Test wider. Obwohl ich in den meisten anderen Bereichen wohl eher den „Mockist“-Weg vorziehe, hatte ich diesbezüglich wohl eher die „klassische“ Denkweise. Das zustandsbasierte Testing passt aber nicht so ganz zum verhaltensorientierten Testing beim mocken.

Man muss natürlic dazu sagen, dass der Artikel etwas älter ist (aber nicht wirklich „veraltet“), er geht auf JMock ein (die alte Version), die felxibler war (nach damaligen Gesichtspunkten):

This can be worsened by the nature of mock toolkits. Often mock tools specify very specific method calls and parameter matches, even when they aren’t relevant to this particular test. One of the aims of the jMock toolkit is to be more flexible in its specification of the expectations to allow expectations to be looser in areas where it doesn’t matter, at the cost of using strings that can make refactoring more tricky.

Denke ihm geht es darum dass die Parameter für die Mock Aufrufe flexbiler gehanghabt werden, um genau zu sein:
Anstatt auch die Werte (Zustände, States) der Mockaufrufe zu prüfen, wird nur geprüft ob die entsprechenden Methoden in der richtigen Reihenfolge aufgerufen wurden (Verhalten/Behaviour).
Das ist natürlich alles etwas länger her (2007) und heute gibt es IMHO eigentlich nur ein Mocking Framework das man nutzen sollte: JMockit
Kann alle dreckigen Tricks (statische Felder mocken, new Operator umbiegen, etc. pp., wichtig bei Legacy Code), aber auch ganz „einfache Dinge“ für normales TDD.

Das Tests beim Refactoring mitgeändert werden müssen ist kar, idealerweise eben für 1 Änderung im Prodcode nur 1 Änderung im testcode.

Wie du bereits sagtest ist DRY extrem wichtig, auch im Testcode.
Stell dir vor du hast 3 Testklassen mit insgesamt ca. 10 Tests, jedes dieser Tests nutzt den Konstruktor von Klasse A direkt. Jetzt änderst du den Konstruktor von Klasse A ab und darfst 3 Klassen/30 Test abändern… die Lösung ist sehr einfach, Factory Methode/Object Mother/Builder, damit muss nur an einer einzigen Stelle in den Tests geändert wenn sich der Konstruktor ändert.
Das ist auch gar nicht so wild, ich ändere zB. einfach so den Konstruktor ab und wenn ich dann sehe dass sehr viel „rot“ wird in den Tests, mache ich das schnell wieder rückgängig und refactore dann die Tests erst.

Ob man seine Tests als Minenfeld oder als Sicherheitsnetz empfindet kommt IME stark darauf an wie man die Tests und den ProdCode schreibt.

Bin ehrlich gesagt die letzten 2 Jahre beruflich auch nicht mehr zu TDD gekommen :frowning: Legacy Code halt, muss mir wohl ein neues Greenfield Projekt suchen…
Wenn man sich mal daran gewöhnt hat tut es richtig weh ohne, keiner macht beim ersten mal alles richtig, ohne den Refactoring Schritt bleibt der erste Entwurf oft der Finale…

Genau dieses Mantra versuche ich mir einzubläuen (darauf wird ja auch überall hingewiesen, wenn man was zum Thema TDD liest). Aber es ist doch vorerst so viel schneller, „mal eben schnell“ dieses oder jenes Feature ohne den Test vorher geschrieben zu haben zu implementieren

Ja, deine Gewohnheit :slight_smile:
Bei TDD kommt oft erst die Frage „Wie würde ich das testen?“ bevor man sich überlegt „Wie würde ich da implementieren?“.
Mein erster Test in einer neuen Testklasse (bevor eine Implementierung/Interface geschrieben ist) lautet in 99% der Fälle
public void first() {...} und ruft nur den Kontruktor auf um eine Instanz zu erstellen, danach kommt die Sache ins Rollen und dieser first Tests wird sehr schnell gelöscht. Man muss nur mal in Fahrt kommen… :wink:

Hier noch ein älter Artikel zum Thema Mocks und TDD: Mock Roles not Objects
Ist nicht Topaktuell aber geht trotzdem auf Dinge wie „Interface Discovery“ ein, ein Punkt der TDD so wertvoll macht weil es damit zum Design Tool wird.

Das Paper habe ich mir jetzt noch nicht durchgelesen, aber die Autoren kamen mir doch gleich bekannt vor :wink:

Allerdings habe ich mir die Präsentation von Gerard Meszaros über sein Buch „xUnit Test Patterns“ auf YouTube angesehen und bei 12:16 steht folgendes ganz unten auf der Folie:

All Unit Test should be Black Box to allow refactoring

Das widerspricht sich mit deiner Aussage, maki. Das Buch habe ich noch nicht gelesen, daher kann ich nicht sagen, was dort detailliert zu diesem Thema steht. Aber trotzdem würde ich gern verstehen, wie sich diese scheinbar gegensätzlichen Aussagen unter einen Hut bringen lassen.

All Unit Test should be Black Box to allow refactoring

Habs gerade im Video gesehen, er sagt aber nix dazu und es ist ca. 0.5 Sekunden eingeblendet :wink:

Aber trotzdem würde ich gern verstehen, wie sich diese scheinbar gegensätzlichen Aussagen unter einen Hut bringen lassen.

Ich sehe da keinen Widerspruch, Test- und ProdCode Refactoring gehen Hand in Hand.