[Java] JUnit tests für void-Methoden

[Toastbrot]

Hallo zusammen,

Disclaimer: Ja es sind Homeworks, die benotet werden

Ich soll insgesamt 4 tests für ein Tetrisspiel schreiben, welches hier auf GitHub verfügbar ist.

Für folgende Methoden soll ich Tests schreiben:

Tetris

• updateGame()

BoardPanel

• isValidAndEmpty()
• addPiece()
• checkLines()

Manche davon sind private, das ist aber erlaubt, abzuändern (ich habs auf default gestellt)

checkLines ist dabei sehr einfach zu testen:
[src=java]
public class BoardPanelTest {
Tetris tetris;
BoardPanel bp = new BoardPanel(tetris);

@Test
public void testCheckLines() {
assertEquals(0, bp.checkLines());
}
}

[/src]

Aber bei den anderen 3 hab ich echt meine Probleme, da sie zum einen void-Methoden sind und zum anderen ich meine dass das Spiel dafür gestartet werden muss, was für automatische Tests ja eher schlecht ist...

Kann mir da jemand weiterhelfen? Bestenfalls einen kurzen Ansatz für die updateGame-Methode, womit ich dann weiterarbeiten kann

Beste Grüße,
Toast

 

[Brother John]

Mit Unit Tests überprüft man nur die Public API. Private Implementierungsdetails sind genau das, was der Name sagt: interne Details und unsichtbar nach außen. Die kann und soll man nicht direkt testen. So gesehen ist die Hausaufgabe – sagen wir – ungünstig strukturiert.

Egal ob Void-Funktion oder nicht, du testest immer die öffentlich sichtbaren Auswirkungen von einem bestimmten Verhalten. Das kann z.B. der Rückgabewert einer Funktion sein. Bei einer Void-Funktion ist es eine Zustandsänderung woanders (ein side effect). Was das genau heißt, hängt von der jeweiligen Funktion ab. Du hast aber immer irgendwo Zustand rumfliegen, den du anschauen und mit deiner Erwartung abgleichen kannst. Wo das nicht der Fall ist, gibts auch nichts zu testen.

Ich hab jetzt nur in Tetris.java für die `updateGame()` reingeschaut. Und naja, das ist … blöd. Du hast die Anforderung, eine Funktion zu testen, die a) eigentlich ein privates Implementierungsdetail ist und b) vermutlich deswegen nicht dafür designt wurde, in Isolation getestet zu werden.

Hilft aber nix. Es bleibt dir nichts anderes übrig als den Quellcode zu lesen und die Zustandsänderungen rauszufinden, die `updateGame()` verursacht. In einem Test bringst du dann erstmal das Spiel in einen definierten Ausgangszustand, rufst die Funktion und prüfst dann, dass alle relevanten Zustände wie erwartet aussehen. Das wird sich nervig und unschön anfühlen. Aber das liegt nicht an dir, sondern daran, dass die Aufgabenstellung »Schreibe einen Unit Test für eine private Funktion« Unsinn ist.

 

[Toastbrot]

Danke für die Antwort. Hat mir auf jeden Fall weitergeholfen, da ist zumindest mal teste, ob currentRow inkrementiert wird.

Aber hier komme ich wieder zum nächsten Problem: Das Programm prüft die vergangene Zeit immer auf die Reale Zeit in Millisekunden. Hier ist es dann möglich, dass der Block in x ms nun um eine Reihe gefallen ist oder halt auch nicht. Sowas ist doch m.E. nicht möglich, konstant zu testen, da das Programm nie konstant gleich schnell abläuft oder? (Vor allem nicht auf unterschiedlichen Rechnern...)

 

[exomo]

In dem Fall ist es wiederum schlechtes Design. (Zumindest in Bezug auf Testbarkeit).
Wenn man die Funktion testbar machen wollte, müsste man die Zeitmessung per Dependency Injection an die Klasse geben, dann könnte man im Test ein Mock-Objekt verwenden.
Code der auf Zeitmessungen beruht ist sonst schwer bis unmöglich sinnvoll zu testen.

Edit: Gerade auch mal in den Code geschaut. Konkret ist das Problem hier:

this.logicTimer = new Clock(gameSpeed);

Die Klasse Clock sollte ein interface (z.B.) IClock implementieren, das die öffentliche Schnittstelle der Clock Klasse definiert. Die Klasse Tetris bekommt im Konstruktor eine IClock Referenz übergeben und arbeitet nur mit diesem Objekt. In der Main Methode kann Tetris dann mit

new Tetris(new Clock())

initialisiert werden, im Test kannst du die Tetris mit einem Mock<IClock> initialisieren. Leider kenne ich mich mit Java nicht gut genug aus, sonst würde ich eventuell einen Beispiel Mock Code aufschreiben.

 

[Brother John]

DI im weiteren Sinne wäre nicht nur für die Testbarkeit gut. Ich hatte das vor kurzem für eine Animationssteuerung so gelöst:
[src=cpp]Position calculateNextPosition(std::chrono::nanoseconds elapsed);[/src]
Der Parameter ist die Zeit seit dem letzten Aufruf dieser Funktion (oder seit dem Start der Animation beim ersten Aufruf). So braucht der Algorithmus intern keine Uhr mehr, sondern addiert nur noch (Pseudo-)Nanosekunden. Wie schnell und gleichmäßig die Uhr läuft, ist komplett extern gesteuert. Im Test macht man dann kurze Nanosekunden – soviel wie die CPU hergibt. Und für den Produktivbetrieb nimmt man eine echte Uhr.

@Toastbrot
OK, zurück zum Thema. Du könntest alle Instanzvariablen public machen und verschiedene komplette Spielzustände faken, so dass sie für verschiedene Testcases passen. Das hat zwar mit sinnvollem Testen nur noch bedingt etwas zu tun, aber außer einem grundlegenden Refactoring sehe ich keine andere Möglichkeit.

Eigentlich bist du gerade in einer sehr praxisnahen Situation. Wenn das Management entscheidet, dass wir jetzt Code-Coverage brauchen, »weil Coverage ist … äh, naja … also Coverage, die … äh … die ist ja schon nötig und … Best Practice und so und … und wir brauchen ne einfache Zahl für den nächsten Satz Powerpoints!« Manchmal kommt man nicht drum rum, mit solchen technisch höchst fundierten Entscheidungen zu leben … Und dann schreibst du halt Unittests für Code, den du im Leben nicht ändern willst, weil, wie Uncle Bob sagt: »If you touch it, you break it. And if you break it, it becomes YOURS.«

Was ich mich insgesamt Frage: Was ist eigentlich der Sinn dieser Hausaufgabe? Hast du wirklich nur die Aufgabenstellung »schreibe einen Test«? Muss der einfach nur irgendwas sinnvolles testen? Musst du eine bestimmte Mindest-Coverage erreichen? Muss es ein klassischer TDD-artiger Unittest sein? Wie genau darfst du den zu testenden Code verändern? Wenn das im Startpost wirklich die ganze Aufgabenstellung ist, würde ich zum Aufgabensteller gehen und nachfragen, was genau die Erwartung ist. Zumindest mir ist das unklar.

 

[Toastbrot]

@Brother John: Die genaue Aufgabenstellung ist folgende:

Spoiler

Create a JUnit test suite, and add both test classes into the test suite. Put all your test cases and
test suite into a different folder (e.g., test). After you finish your assignment, add your project
into a zip file.
In addition, you need to write a report. The report should include the following content.

1. What problem(s) did you find in the code? For each problem, further explain how you
   found it (e.g., using which test case).
2. Specifically explain the test case that you have created for the updateGame method of
   Class Tetris. What is your input, and what is your expected output? What is your logic of
   testing this method?
3. Include a screenshot of the result of running your test suite.

Den vom Prof gestellten Code hatte einen eingebauten Fehler, den ich mit Aufgabe 1 bereits gefunden und durch einen Test teste.

In Aufgabe 2 kann ich ja argumentieren, was alles so falsch ist und was gegen die best practices verstößt, etc.., also alles was ihr bisher an der Aufgabe kritisiert habt
Das wäre natürlich die Alternative: Anstatt unnötig komplizierte Tests zu schreiben, eher mal argumentieren, warum das hier nicht sinnvoll ist.

 

[Brother John]

Bauchgefühl: Die insgesamte schlechte Testbarkeit zu erkennen und anzusprechen klingt nicht nach Teil der Aufgabe. Der Code ist also eher nicht bewusst suboptimal geschrieben. Das macht intensive Kritik schwierig, weil du damit extrem schnell Egos ankratzt. Ich kenne den Prof natürlich nicht, aber dieses Wespennest will auf jeden Fall gut überlegt sein.

 

[BurnerR]

Man ist gut beraten, seine Meinung nicht in die Lösung rein zu kodieren oder gar explizit zu machen, sondern komplett außen vor zu lassen.
Du kannst aber rein sachlich beschreiben was du testen wolltest, welche Probleme es dabei gegeben hat (Punkt 1) und wie du den das Problem dann gelöst hast (Punkt 2) aka den Code dann geändert hast (Dependency Injection / Interfaces / ..). Im Idealfall indem du noch entsprechende Literatur mit anführst (GoF, PEAA, ..) - aber das führt vermutlich zu weit.

Für die Aufgabe evtl. egal, aber theoretisch beißt sich das ja in den Schwanz, denn bevor du den Code ändern kannst musst du Tests schreiben um sicherzustellen, dass du die Semantik nicht änderst.. .

 

[Toastbrot]

Okay, dann werde ich wohl doch nciht kritisieren. Aber ich werde auf jeden Fall aufzeigen, auf welche Probleme ich dabei gestoßen bin, dass die originale Klasse nicht testbar ist und es Probleme mit dem Timer gibt.
Habe aber inzwischen alle Tests soweit hinbekommen dass sie konstant alle durchlaufen, auch wenn ich wirklich viele Variablen public machen musste und sogar die Methode Tetris.startGame() komplett in meine Testklasse kopieren und zerlegen musste... Sauber ist anders

Danke euch auf jeden Fall für das ganze Feedback, hat mich auf jeden Fall weiter gebracht und meine ersten Befürchtungen auch unterstützt.


Btw. das Spiel ist nicht vom Prof selbst, wird aber in ähnlichen Kursen von anderen Profs oftmals genutzt, so wie ich das bereits ergoogelt habe. Aber das ist hier eh ziemlicher Standard, dass fast alles von anderen Professoren kopiert wird und man dadurch auch oft sogar für Klausuren im Internet vollständige Lösungen findet.

 

[BurnerR]

Profs sind generell oft keine Programmierer / Entwickler und wenn sie es doch mal waren, dann meistens vor 20 Jahren oder länger. Alles andere haben die sich auch nur angelesen im Idealfall. Ich finde es eher sehr löblich, dass bei euch augenscheinlich relativ viel Augenmerk auf Testing gelegt wird. Das wird gerne mal komplett ausgeklammert.