UnitTesting wieviel testen?

[ehc_master]

Hi, ich bin gerade dabei, mir Unittesting genauer anzuschauen.
Wieviel Testing machen eigentlich Sinn? Hier mal ein paar Beispiele bzw Fragen dazu:

Erstes Beispiel
GetAll-Funktion von meinem ORM.
Ich teste hier:
- ob das result not empty ist
- ob die Anzahl mit dem Fixture übereinstimmt (damit kann ich den test davor entfernen oder?)
- ob ein item vom resultarray die richtige klasse hat (assertContainsOnlyInstancesOf)
was würde man noch testen oder ist das so ok?

Zweites Beispiel als Code

    public function testToArrayWithId()
    {
        $objectToArray = $this->getOne();
        $array         = $objectToArray->toArray();
        $this->assertArrayHasKey('id', $array);
        $this->assertArrayHasKey('name', $array);
        $this->assertArrayHasKey('wert', $array);
        $this->assertArrayHasKey('bezeichnung', $array);
        $this->assertArrayNotHasKey('db', $array);
        $this->assertArrayNotHasKey('tableName', $array);
    }

    public function testToArrayWithoutId()
    {
        $objectToArray  = $this->getOne();
        $arrayWithoutId = $objectToArray->toArray(false);
        $this->assertArrayNotHasKey('id', $arrayWithoutId);
    } 

ist sowas ok oder mist?

Drittes Beispiel
wenn ich eine löschenfunktion habe
teste ich dann den count in der db von vorher und nachher
oder vergleiche ich die anzahl zwischen db und fixture, beides vorher und beides nachher oder reicht der unterschied zwischen beiden am ende?

Viertes Beispiel
dann habe ich noch eine save funktion die je nach id ein update oder insert ausführt. schreibt man hier für alle 3 funktionen tests, sicher oder ?

Fünftes Beispiel
Wenn ich in einer Modelklasse immer wieder ein Objekt benötige, kann ich das einmalig in getConnections ausführen?

Sechstes Beispiel
Ich habe eine abstrakte Klasse, die eine leere protected queryPre- und queryPost-Funktion hat, wie kann ich sowas testen ??
Ich kann problemlos über die erbende Klasse die Funktionen der abstrakten Klasse testen (erkennt codecoverage), aber wie siehts mit ner leeren funktion aus ? ^^

Siebentens Beispiel
ist sowas ok/normal?

/**
     * @covers ::save
     * @covers ::insert
     * @covers ::queryPre
     * @covers ::query
     * @covers ::queryPost
    */
    public function testSaveNew()
    {
      
    }

    /**
     * @covers ::save
     * @covers ::update
     * @covers ::query
     */
    public function testSaveOld() 

Achtes Beispiel
Wenn ich eine abstrakte klasse habe und mit den unterklassen teile der oberklasse teste, kann ich dann für die testklassen dann auch eine übergeordnete machen, um die tests nur einmal zu schreiben und jedesmal mit auszuführen, oder ist das quatsch? wenn quatsch, wie markiere ich für mich den test, das hier auch die oberklasse getestet wird?
€dit: hab grad gelesen, dass es sinn macht und man direkt die getConnctions abstract setzen und die tests davon erben sollen

Neuntes Beispiel
Pro Klasse gibt es Mindestens eine Testfile. Wie kann ich die selbe DB verbindung nutzen, oder sollte man daran garnicht denken? Baut ihr die bei jeder Testklasse neu auf?

btw
vlt könnt ihr mal schreiben, was ihr beim standard-CRUD testet

Status
nach ca 1tag sieht es so aus phpunit.jpg



Next
sobald alle funktionen getestet sind, schaue ich mir optimierungen der tests an, aktuell brauch er für 29 Tests / 106 asserts knapp 9 Sekunden. Habe aber auch noch keine iteratorklasse, abstr.testklasse, globals, usw. verwendet

[Andreas]

1.) Üblicherweise achtet man bei Unit-Tests darauf, dass sie isoliert ablaufen (Unit-Test, also nicht mehrere Klassen auf einmal oder Abhängigkeiten, sondern EINE Klasse). Sprich: Man bindet einen Unit-Test nicht an eine echte Datenbank an, sondern arbeitet mit Mocks. Das was du machst, nennt man bestenfalls Integrationstests (die man aber auch mit PHPUnit machen kann)

Ein Beispiel:

<?php
// ...
public function testGetAll() {
    $dbMock = $this->getMockBuilder(DBConnection::class)->setMethods(["query", "fetchAll"])->disableOriginalContructor()->getMock();
    $dbMock->expects($this->once())->method("query")->with("select * from users")->willReturn("fake-result");
    $dbMock->expects($this->once())->method("fetchAll")->with("fake-result")->willReturn(["fake-user-1", "fake-user-2"]);
    $orm = new ORM($dbMock);
    $actual = $orm->getAll('user');
    $expected = [["fake-user-1"], ["fake-user-2"]];
    $this->assertEquals($actual, $expected);
}

2.) Auch hier gilt wieder: Mocken statt echte Datenbank verwenden. Was man auch machen kann, wenn man eine echte DB anbinden muss, ist eine sqlite-Datenbank im Arbeitsspeicher zu erzeugen und anzubinden, die dann nach Script-Ablauf einfach wieder verworfen wird.

Kleine Kritik an deiner Methode:
Warum implementierst du kein ArrayAccess-Interface und sparst dir den ganzen Konvertierungsaufwand?
Methoden mit Bool-Parametern sind meines Erachtens nach meistens Mist... in diesem Fall auch (Methoden sollten nur eine Aufgabe erfüllen...)

<?php
$objectToArray->toArray(false);

3.) Man testet nicht den Count... wenn dann die Daten. Woher willst du sonst wissen, dass dein Code nicht etwas anderes gelöscht hat, als vorgesehen war... Aber grundsätzlich das selbe wie in allen anderen Fällen. Mocks nutzen.

4 und 5 lass ich mal weg... das selbe in grün

6.)

Ich habe eine abstrakte Klasse, die eine leere protected queryPre- und queryPost-Funktion hat, wie kann ich sowas testen ??

Du solltest dich fragen, ob leere Methoden Sinn machen. Falls ja (wäre tatsächlich möglich), dann sind sie nur schwer testbar, da sie keine Daten Ändern und keine Methoden aufrufen.
Möglich wäre:

<?php

// ...
public function queryPre() {
    foreach($this->queryPreHandlers as $handler) {
        $handler->execute($this);
    }
}

Dann könntest du das $this->queryPreHandlers mit Mocks füllen und es so testen. Wenn die Methode nix macht, wirds schwierig.

7.) Sieht nicht nach einem üblichen Pattern aus

8.)

Wenn ich eine abstrakte klasse habe und mit den unterklassen teile der oberklasse teste, kann ich dann für die testklassen dann auch eine übergeordnete machen, um die tests nur einmal zu schreiben und jedesmal mit auszuführen, oder ist das quatsch?

Tests zu strukturieren und doppelten Code auch hier zu vermeiden, ist sogar sehr gut. Allerdings bezweifle ich, dass die Tests einer Oberklasse in einer Unterklasse noch mal gemacht werden müssen. Wenn Code einmal getestet ist und nicht dupliziert wurde (was du ja per se mit Abstract classes vermeiden willst), testest du es auch nur da, wo es implementiert wurde.
Du kannst abstrakte Klassen mit ->getMockForAbstractClass sehr gut testen. Traits gehen auch mit ->getMockForTrait.

vlt könnt ihr mal schreiben, was ihr beim standard-CRUD testet

Ich arbeite gerade an einem Test-Driven-Development-Projekt. Dabei ist so gut wie alles getestet. Vom Anlegen einer Datenbank (hier verwende ich wie erwähnt sqllite-In-Memory-Datenbanken), über die Persistenz (Mit Mocks des DBAL), aber auch die Service-Provider (ich verwende dieses Pattern sehr gerne, siehe z.B. Pimple).


Dann noch ein paar Tipps:
- Code-Coverage ist sehr gut. Ich persönlich finde es unerlässlich, da man wissen sollte, welche Teile des Codes abgedeckt sind, und welche nicht. Als Richtwert sollte man mindestens 80% anstreben.
- Deine Tests dauern so lange, weil du die echte Datenbank dranhängst. Ich würde die Unit-Tests isolieren und Integrationstests machen, die mit einer In-Memory-Datenbank arbeiten (sofern du ein DBAL nutzt)
- Dein Code wirkt zwar durchdacht, aber nicht besonders professionell. Schau dir mal aktuelle Frameworks an, wie die sowas machen (UND vor allem testen). Doctrine zum Beispiel.


Ich hoffe, das hilft dir jetzt*g*
Viele Grüße

[ehc_master]

Danke für dein Feedback!
Ich hab gestern nur ein paar Mocks eingesetzt und das noch mit der alten Funktion. Werde das heute umbauen, den helper/builder nutzen und das u.a. auch für die db.
Vom getMockForAbstractClass hatte ich gestern auch gelesen, werde ich mir heute auch nochmal genauer anschauen.
Die leere Funktion habe ich mit einer erbenden Klasse getestet, die die Funktion überschreibt/befüllt, aber mit dem handler ist auch ne schöne Idee(nehm ich mit in meine Todos). Ich komm bloß aktuell mitn Refactoring nicht mehr hinterher Gerade zuviel im Reallife los ^^

[ehc_master]

ich muss nochmal fragen:
also zum testen einer orm-klasse teste ich dann nur einmal die pdo-verbindung und ansonsten benutze ich immer ein pdo-mock?
lager ich den pdo-test aus oder mache ich den test in jedem test pro orm-klasse (zb mit der abstracten klasse)?
wenn ich nur diese unit teste, gehe ich davon aus, das andere units/mocks funktionieren wie sie sollen?

sry bin noch am anfang mit unittests und muss erstmal meine struktur/aufbau der tests finden/erarbeiten

[Marlon Schultz]

Ein Beispiel:

<?php
// ...
public function testGetAll() {
$dbMock = $this->getMockBuilder(DBConnection::class)->setMethods(["query", "fetchAll"])->disableOriginalContructor()->getMock();
$dbMock->expects($this->once())->method("query")->with("select * from users")->willReturn("fake-result");
$dbMock->expects($this->once())->method("fetchAll")->with("fake-result")->willReturn(["fake-user-1", "fake-user-2"]);
$orm = new ORM($dbMock);
$actual = $orm->getAll('user');
$expected = [["fake-user-1"], ["fake-user-2"]];
$this->assertEquals($actual, $expected);
}

Der Test testet nur ob Du richtig gemockt hast. Inhaltlich bringt das mE gar nichts. Wenn dabei nicht noch irgendeine Logik dahinter her kommt, wo auch was mit den Daten geschieht, dann kann sich solche Tests sparen.

[Andreas]

Der Test testet nur ob Du richtig gemockt hast. Inhaltlich bringt das mE gar nichts. Wenn dabei nicht noch irgendeine Logik dahinter her kommt, wo auch was mit den Daten geschieht, dann kann sich solche Tests sparen.

Das sehe ich etwas anders... der Test testet, ob die Methoden in der richtigen Reihenfolge mit den richtigen Argumenten aufgerufen werden, den richtigen Rückgabewert haben und eine Umwandlung von ["fake-user-1", "fake-user-2"] nach [["fake-user-1"], ["fake-user-2"]] erfolgt. Nur dann gilt die Logik als korrekt ausgeführt. Natürlich könnte man bei $expected auch ein anderes Ergebnis prüfen, z.B. ein array von Model-Objekten. Das war mir nur zu viel zu schreiben

also zum testen einer orm-klasse teste ich dann nur einmal die pdo-verbindung und ansonsten benutze ich immer ein pdo-mock?

Wenn du mit PDO arbeitest, wirst du ja irgendwie das PDO-Objekt an dein ORM-System übergeben, damit das ORM die Queries ausführen kann, die es braucht.
z.B.:

<?php
$db = new PDO(...);
$orm = new Orm($db);
$users = $orm->getAll('user');

In den privaten Methoden des ORM arbeitest du dann nur noch mit dem PDO-Objekt. Sicherzustellen beim Mocken ist also, dass die Methoden des PDO-Objektes in der richtigen Reihenfolge und mit den richtigen Parametern aufgerufen werden, und das zurück geben, was das "echte" PDO-Objekt zurück geben würde. Z.B.:

<?php
// ...
public function getAll($type) {
    $sql = 'SELECT * FROM '.$type.'';
    foreach ($conn->query($sql) as $row) {
        // tatsächliche User-Models zusammenbauen
    }
    // tatsächliche User-Models zurück geben
}

// test dazu:
public function testGetAll() {
    $dbMock = $this->getMockBuilder(PDO::class)->setMethods(["query"])->disableOriginalContructor()->getMock();
    $dbMock->expects($this->once())->method("query")->with("select * from users")->willReturn([["username" => "john", "firstname" => "John", "lastname" => "Doe"]]);
    $orm = new ORM($dbMock);
    $actual = $orm->getAll('user');
    $expected = [ /** Tatsächliche User-Models **/];
    $this->assertEquals($actual, $expected);
}

Ein Integrationstest könnte z.B. so aussehen:

<?php

public static function setUpBeforeClass() {
    self::$pdo = new PDO( 'sqlite::memory:', null, null, array(PDO::ATTR_PERSISTENT => true));
    self::$pdo->exec("create database integration_tests");
    self::$pdo->exec("use integration_tests");
    self::$pdo->exec("create table users");
    self::$pdo->exec("insert into users (username, firstname, lastname) values('john', 'John', 'Doe')");
}

public static function tearDownAfterClass() {
     self::$pdo->exec("drop database integration_tests");
     self::$pdo->closeCursor();
}

public function testGetAll() {
    $orm = new ORM(self::$pdo);
    $actual = $orm->getAll('user');
    $expected = [ /** Tatsächliche User-Models **/];
    $this->assertEquals($actual, $expected);
}

Wichtige Info: Den Code habe ich nur so hingehackt und nicht getestet... ist nur ein Beispiel um das Prinzip zu erklären.