Code-Review für Tests: Worauf besonders zu achten ist

1. Warum Testcode im Review oft weniger Aufmerksamkeit bekommt

Reviewer konzentrieren sich naturgemäß zuerst auf die Produktivlogik eines Pull Requests, da hier vermeintlich das eigentliche fachliche Risiko liegt, während der begleitende Testcode eher als bestätigender Beleg dafür wahrgenommen wird, dass die Produktivlogik bereits korrekt funktioniert, statt als eigenständiges Artefakt, das genauso sorgfältig auf Korrektheit und Sinnhaftigkeit geprüft werden müsste.

Diese Wahrnehmung ist trüglich, denn ein fehlerhafter oder bedeutungsloser Test ist in gewisser Weise gefährlicher als gar kein Test, da er den Anschein von Absicherung erweckt, ohne diese Absicherung tatsächlich zu leisten, wodurch sich ein Team fälschlich in Sicherheit wiegt und wichtige, manuelle Zusatzprüfungen unterlässt, die es bei erkennbar fehlender Testabdeckung vermutlich durchgeführt hätte.

2. Assertions inhaltlich prüfen statt nur Struktur abnicken

Ein häufiger, oberflächlicher Reviewfehler besteht darin, lediglich zu prüfen, ob ein Test formal vorhanden, benannt und lauffähig ist, ohne tatsächlich zu lesen, was die enthaltenen Assertions konkret behaupten und ob diese Behauptung fachlich zum getesteten Verhalten passt. Ein Test kann syntaktisch vollkommen korrekt sein und trotzdem eine viel zu schwache oder schlicht falsche Aussage treffen, etwa indem er nur prüft, dass eine Funktion nicht wirft, statt zu prüfen, dass sie das fachlich korrekte Ergebnis liefert.

Eine bewährte Reviewfrage lautet deshalb: "Würde dieser Test tatsächlich fehlschlagen, wenn die Produktivlogik den beschriebenen Bug enthielte?" Diese Frage zwingt den Reviewer, die Assertion gedanklich gegen ein konkretes, plausibles Fehlerszenario durchzuspielen, statt sich mit der blossen Existenz einer Assertion zufriedenzugeben.

// SCHWACHE ASSERTION: prüft nur, dass überhaupt etwas zurückkommt
test('berechnet Versandkosten', () => {
  const result = calculateShipping(cart);
  expect(result).toBeDefined();
});

// AUSSAGEKRAEFTIGE ASSERTION: prüft den tatsächlich erwarteten Wert
test('berechnet Versandkosten für Standardversand unter Freigrenze', () => {
  const cart = { items: [{ price: 30 }], weight: 2 };
  const result = calculateShipping(cart);
  expect(result).toEqual({ method: 'standard', cost: 4.99 });
});

3. Typische Review-Fehler bei Testcode im Überblick

Neben zu schwachen Assertions gehört das Uebersehen fehlender Negativ-Fälle zu den häufigsten Review-Fehlern: Ein Pull Request, der nur den erfolgreichen Pfad einer neuen Funktion testet, aber keinen einzigen Test für ungültige Eingaben, leere Ergebnismengen oder Randfälle enthält, wird im Review oft trotzdem akzeptiert, weil der vorhandene, grüne Test optisch bereits Vertrauen erweckt.

Ein weiterer verbreiteter Fehler ist, Testnamen nicht kritisch zu hinterfragen: Ein Testname wie `testFunktioniert()` oder `testCase1()` verrentet keinerlei Information darüber, welches Verhalten tatsächlich geprüft wird, und sollte im Review konsequent zurückgewiesen werden, zugunsten eines Namens, der die geprüfte Bedingung und das erwartete Ergebnis in verständlicher Sprache benennt, etwa `testWirftAusnahmeBeiNegativemBetrag()`.

Ebenfalls übersehen wird häufig eine unzureichende Testisolation, etwa wenn ein Test auf globalem, geteiltem Zustand aufbaut oder von der Ausführungsreihenfolge anderer Tests abhängt, was im Review nur auffällt, wenn der Reviewer den Test bewusst auf Abhängigkeiten von äußerem Zustand hin durchsieht, statt sich allein auf einen erfolgreichen CI-Lauf zu verlassen.

4. Flakiness-Risiken bereits im Review erkennen

Ein aufmerksamer Reviewer kann viele später auftretende, flakige Tests bereits im Review erkennen, indem er gezielt nach bestimmten Warnsignalen sucht: feste Wartezeiten (`sleep(2000)`) statt bedingtem Warten auf einen konkreten Zustand, Assertions auf exakte Zeitstempel oder auf die Reihenfolge asynchron ablaufender Operationen, sowie Tests, die auf externe, nicht vom Test selbst kontrollierte Ressourcen wie das aktuelle Datum oder einen Zufallsgenerator ohne festen Seed zugreifen.

Ein weiteres, im Review erkennbares Flakiness-Risiko ist mangelnde Aufräum-Logik: Ein Test, der Daten in einer gemeinsam genutzten Testdatenbank anlegt, aber am Ende nicht zuverlässig wieder entfernt, kann nachfolgende, eigentlich unabhängige Tests verunreinigen und dadurch scheinbar zufällige, schwer reproduzierbare Fehlschläge in ganz anderen Testdateien verursachen. Reviewer sollten deshalb gezielt prüfen, ob ein neuer Test entweder in einer vollständig isolierten Transaktion läuft, die am Ende zurückgerollt wird, oder ob er in einem expliziten Teardown-Schritt sämtliche selbst angelegten Daten wieder entfernt.

5. Eine praktische Checkliste für das Testcode-Review

Eine kurze, im Team abgestimmte Checkliste hilft, die genannten Risiken auch unter Zeitdruck nicht zu übersehen: Prüfen der Assertion gegen ein konkretes Fehlerszenario, Prüfen auf fehlende Negativ- und Randfälle, Prüfen des Testnamens auf tatsächliche Aussagekraft, Prüfen auf feste Wartezeiten oder unkontrollierte Zeit-/Zufallsabhängigkeiten, sowie Prüfen der Testisolation und Aufräum-Logik.

Diese Checkliste sollte nicht als starres, bürokratisches Ritual verstanden werden, sondern als Gedächtnisstütze für genau die Aspekte, die unter Zeitdruck am ehesten übersehen werden, und lässt sich in vielen Projekten direkt als PR-Vorlage im Git-Hosting-System hinterlegen, sodass sie bei jedem neuen Pull Request automatisch als Erinnerung erscheint.

6. Besonderheiten beim Review von Magento- und Hyvä-Tests

In einem Magento-Projekt mit Hyvä-Frontend sollten Reviewer bei PHPUnit-Integrationstests zusätzlich prüfen, ob ein Test tatsächlich gegen die richtige Fixture-Store-Konfiguration läuft, etwa über `#[DataFixture]`- oder `#[ConfigFixture]`-Attribute, da ein Test, der versehentlich gegen die Standard-Store-Konfiguration statt einer bewusst vorbereiteten Testkonfiguration läuft, unbemerkt fachlich falsche Annahmen treffen kann.

Bei Playwright- oder Cypress-Tests für das Hyvä-Frontend lohnt sich außerdem ein gezielter Blick darauf, ob Selektoren stabil über `data-testid`-Attribute erfolgen statt über Tailwind-Klassennamen, da Letztere sich bei jedem CSS-Refactoring ändern können und dadurch Tests unnötig brückig gegenüber rein optischen Anpassungen machen, die fachlich gar keine Aenderung darstellen.

7. Eine Kultur etablieren, in der Testcode-Kommentare ernst genommen werden

Damit Reviewer tatsächlich Zeit in ein gründliches Testcode-Review investieren, muss diese Sorgfalt im Team sichtbar wertgeschätzt werden, etwa indem ein besonders präzise formulierter Testfall im Review explizit positiv hervorgehoben wird, statt Testcode-Kommentare implizit als weniger wichtig als Kommentare zur Produktivlogik zu behandeln.

Ebenso hilfreich ist, wiederkehrende Testcode-Probleme, etwa ein bestimmtes Flakiness-Muster, das mehrfach im Review aufgefallen ist, in einer gemeinsamen, kurzen Team-Dokumentation festzuhalten, damit neue Kolleginnen und Kollegen von bereits gesammelten Erfahrungen profitieren, statt dieselben Fehler erneut zu machen und im Review erneut korrigiert werden zu müssen.

8. Automatisierte Vorprüfungen vor dem menschlichen Review

Ein großer Teil der zuvor genannten Reviewfehler lässt sich bereits vor dem eigentlichen menschlichen Review automatisiert abfangen, wodurch der Reviewer seine begrenzte Aufmerksamkeit auf die tatsächlich fachliche Bewertung der Assertions konzentrieren kann, statt sie mit mechanisch prüfbaren Formalitäten zu verbrauchen. Ein eigener PHPCS-Sniff, der generische Testnamen wie `test1` oder `testFunktioniert` bereits in der CI-Pipeline zurückweist, verhindert zuverlässig, dass solche Namen überhaupt erst in einen Pull Request gelangen, statt sich auf die Aufmerksamkeit jedes einzelnen Reviewers zu verlassen.

Auch eine automatisierte Flakiness-Erkennung lässt sich vor das menschliche Review schalten: Ein CI-System, das jeden neuen Test mehrfach hintereinander ausführt und bei inkonsistenten Ergebnissen automatisch einen Kommentar im Pull Request hinterlässt, macht ein potenzielles Flakiness-Risiko sichtbar, bevor der Reviewer überhaupt mit dem inhaltlichen Lesen beginnt, und verhindert dadurch, dass ein subtiles Zeit- oder Reihenfolgeproblem im Trubel eines großen Pull Requests übersehen wird.

Für besonders kritische Module lohnt sich zusätzlich ein automatisierter Mutation-Score als Merge-Voraussetzung: Fällt der Mutation-Score eines betroffenen Moduls durch einen neuen Pull Request unter einen zuvor festgelegten Schwellenwert, blockiert die Pipeline den Merge automatisch, bis entweder stärkere Assertions nachgereicht oder die Abweichung bewusst und dokumentiert akzeptiert wird, wodurch schwache Assertions nicht mehr allein von der Wachsamkeit des menschlichen Reviewers abhängen.

9. Review-Schwerpunkte im Überblick

Die folgende Tabelle fasst die wichtigsten Schwerpunkte für ein gründliches Testcode-Review zusammen.

10. Zusammenfassung

11. FAQ: Code-Review für Tests: Das Wichtigste auf einen Blick