Unit Tests, Fabriken und das Gesetz von Demeter

8

So funktioniert mein Code. Ich habe ein Objekt, das den aktuellen Status einer Warenkorbbestellung darstellt und in einer Einkaufs-API eines Drittanbieters gespeichert ist. In meinem Controller-Code möchte ich Folgendes aufrufen können:

myOrder.updateQuantity(2);

Um die Nachricht tatsächlich an den Dritten zu senden, muss der Dritte auch einige Dinge wissen, die für DIESE Bestellung spezifisch sind, wie das orderIDund das loginID, die sich während der Lebensdauer der Anwendung nicht ändern.

Wenn ich also myOrderursprünglich erstelle , injiziere ich ein MessageFactory, das weiß loginID. Wenn updateQuantitydann gerufen wird, Ordergeht das weiter orderID. Der Steuercode ist einfach zu schreiben. Ein anderer Thread behandelt den Rückruf und aktualisiert, Orderwenn die Änderung erfolgreich war, oder informiert, Orderdass die Änderung fehlgeschlagen ist, wenn dies nicht der Fall war.

Das Problem ist das Testen. Da das OrderObjekt von a abhängt MessageFactoryund MessageFactorytatsächliche Messages zurückgeben muss (die es beispielsweise aufruft .setOrderID()), muss ich jetzt sehr komplizierte MessageFactoryMocks einrichten . Außerdem möchte ich keine Feen töten, da "Jedes Mal, wenn ein Mock einen Mock zurückgibt, stirbt eine Fee."

Wie kann ich dieses Problem lösen, während der Controller-Code genauso einfach bleibt? Ich habe diese Frage gelesen: /programming/791940/law-of-demeter-on-factory-pattern-and-dependency-injection, aber es hat nicht geholfen, weil es nicht über das Testproblem gesprochen hat .

Einige Lösungen, an die ich gedacht habe:

  1. Refaktorieren Sie den Code irgendwie so, dass die Factory-Methode keine realen Objekte zurückgibt. Vielleicht ist es weniger eine Fabrik und mehr ein MessageSender?
  2. Erstellen Sie eine Nur-Test-Implementierung von MessageFactoryund fügen Sie diese ein.

Der Code ist ziemlich kompliziert, hier ist mein Versuch eines sscce:

public class Order implements UpdateHandler {
    private final MessageFactory factory;
    private final MessageLayer layer;

    private OrderData data;

    // Package private constructor, this should only be called by the OrderBuilder object.
    Order(OrderBuilder builder, OrderData initial) {
        this.factory = builder.getFactory();
        this.layer = builder.getLayer();
        this.data = original;
    }

    // Lots of methods like this
    public String getItemID() {
        return data.getItemID();
    }

    // Returns true if the message was placed in the outgoing network queue successfully. Doesn't block for receipt, though.
    public boolean updateQuantity(int newQuantity) {
        Message newMessage = factory.createOrderModification(messageInfo);

        // *** THIS IS THE KEY LINE ***
        // throws an NPE if factory is a mock.
        newMessage.setQuantity(newQuantity); 

        return layer.send(newMessage); 
    }

    // from interface UpdateHandler
    // gets called asynchronously
    @Override 
    public handleUpdate(OrderUpdate update) {
        messageInfo.handleUpdate(update);
    }
}
durron597
quelle
Ich denke, Sie müssen uns den relevanten Code zeigen, den Sie für das OrderObjekt und das Objekt geschrieben haben MessageFactory. Dies ist eine gute Beschreibung, aber es ist etwas abstrakt, direkt mit einer klaren Antwort zu sprechen.
Robert Harvey
@ RobertHarvey Ich hoffe, dass das Update hilft.
Durron597
Versuchen Sie zu überprüfen, ob layer.senddie Nachricht gesendet wurde oder ob die richtige Nachricht gesendet wurde?
Robert Harvey
@RobertHarvey Ich habe darüber nachgedacht, anzurufen verify(messageMock).setQuantity(2), und verify(layer).send(messageMock);auch das updateQuantitysollte false zurückgeben, wenn das OrderUpdate bereits aussteht, aber ich habe diesen Code aus sscce-Gründen weggelassen.
Durron597
3
Ich muss mir Ihren Code noch etwas genauer ansehen ... Aber für die Aufzeichnung halte ich es nicht für eine große Sache, einen Schein zurückzugeben. Die Komplexität von scheinbasierten Tests ist eine unvermeidbare Folge des Umgangs mit veränderlichen Objekten, und da ein Schein keinen Einfluss auf Ihre endgültig veröffentlichte Anwendung hat, sehe ich nicht, wie die Rückkehr eines Kätzchens oder gar Feen tötet.
Robert Harvey

Antworten:

12

Das Hauptproblem hierbei ist, dass Mocks keine Mocks zurückgeben können (oder sollten). Dies ist wahrscheinlich ein guter Rat, spricht aber von einer Lösung: Geben Sie eine echte zurück Message. Wenn die MessageKlasse gut getestet und bestanden ist, können Sie sie als genauso freundlich wie einen Schein betrachten. Vielleicht ist es sogar freundlicher, da es wie das Reale reagiert, weil es das Reale ist.

Welche Art von Real Messagekönnen Sie zurückgeben? Nun, Sie können ein vollwertiges Real Message, ein vereinfachtes Real Message(wobei bekannte Standardeinstellungen verwendet werden) oder ein NullMessage(wie im Null-Objektmuster) zurückgeben. A NullMessageist genauso gültig Messagewie jedes andere und kann an einer anderen Stelle in Ihrer Anwendung abgelegt werden. Welche verwendet werden soll, hängt von der Komplexität des Erstellens und Zurückgebens einer vollständigen Nachricht ab.

In Bezug auf das Gesetz von Demeter gibt es hier mehrere Bedenken. Zuerst verwendet Ihr Konstruktor einen eigenen Builder als Parameter und extrahiert dann Elemente daraus. Dies ist eine eindeutige Verletzung von Demeter und führt auch zu einer überflüssigen Abhängigkeit. Schlimmer noch, der Builder fungiert als Mini-Service-Locator und maskiert die tatsächlichen Abhängigkeiten der Klasse. Sie OrderBuildersollten diese Objekte erstellen und als eigene Parameter übergeben.

Um dies zu testen, würden Sie einen Mock übergeben MessageFactory, der einen Real Message(entweder vollständig, einfach oder null) und einen Mock zurückgibt MessageLayer, der die Nachricht entgegennimmt. Wenn Sie eine vollständige oder vereinfachte Version verwenden Message, können Sie sie von Ihrem MessageLayerModell zurückerhalten und auf Testprüfungen überprüfen.

Ich würde auch das MessageFactoryund MessageLayerals einen Funktionsklumpen auf einer anderen Abstraktionsebene betrachten und so eine MessageSenderKlasse extrahieren , die diese Funktionalität einkapselt. Sie können diese Klasse mit einem einfachen Modell testen MessageSenderund alles, worüber ich oben gesprochen habe, in die MessageSenderTests des Tests verschieben, wodurch Sie sich auch enger an die Einzelverantwortung halten.


Ich sehe, dass es hier wirklich zwei Fragen gibt. Es gibt eine spezielle Frage zum Testen dieses Codes und eine allgemeine Frage zu Mocks, die Mocks zurückgeben. Die spezifische Frage ist die, mit der ich mich oben in größerem Umfang befasst habe, und ich habe am Ende hier mehr Gedanken darüber, nachdem einige weitere Details ans Licht gekommen sind, aber es gibt noch keine wirklich gute Antwort auf die allgemeine Frage: Warum sollten Mocks keine Mocks zurückgeben?

Der Grund, warum Mocks keine Mocks zurückgeben sollten, ist, dass Sie am Ende Ihre Tests testen können, anstatt Ihren Code zu testen. Anstatt nur sicherzustellen, dass das Gerät voll funktionsfähig ist, hängt der Test jetzt von einem völlig neuen Code ab, der nur im Testfall selbst gefunden wird (der selbst oft nicht getestet wird). Dies schafft zwei Probleme.

Erstens kann der Test jetzt nicht sicher sagen, ob das Gerät defekt ist oder ob die miteinander verbundenen Mocks defekt sind. Der springende Punkt eines Tests ist die Schaffung einer isolierten Umgebung, in der es nur eine Fehlerursache geben sollte. Ein Modell für sich allein ist im Allgemeinen sehr einfach und kann direkt auf Probleme überprüft werden. Die Verdrahtung mehrerer Mocks auf diese Weise wird jedoch durch Inspektion exponentiell schwieriger zu bestätigen.

Das zweite Problem ist, dass sich die APIs für die realen Objekte ändern und Tests weit entfernt fehlschlagen können, da sich auch die Mocks nicht automatisch ändern. Hier kommt das Gesetz von Demeter ins Spiel, da dies genau die Art von Effekten ist, die das Gesetz vermeidet. Bei meinen Tests müsste ich mir Sorgen machen, dass nicht nur die Verspottungen direkter Abhängigkeiten, sondern auch die Verspottungen von Abhängigkeiten ad infinitum synchron bleiben . Dies hat den Effekt einer Schrotflintenoperation auf die Tests, wenn sich die Klassen ändern.


Lassen Sie uns nun einige Annahmen bezüglich der spezifischen Frage, wie dieser bestimmte Code getestet werden soll, aufschlüsseln.

Frage 1: Was testen wir wirklich? Während dies ein abgekürzter Teil des Codes ist, können wir hier drei wesentliche Aktivitäten sehen. Erstens haben wir eine Fabrik, die a erzeugt Message. Wir testen nicht, ob die Fabrik das produziert Message, da Sie das bereits verspotten. Wir testen das nicht Message, wie es an anderer Stelle getestet werden sollte, vermutlich in einer Reihe von Tests für die Drittanbieter-API, die das generiert Message. In der zweiten Zeile können wir anhand der Inspektion sehen, dass die Methode einfach auf dem aufgerufen wird Messageund es in der zweiten Zeile wirklich nichts zu testen gibt. Wieder sollte es an anderer Stelle Tests geben, die das Testen überflüssig machen. Die dritte Zeile ruft die MessageLayerAPI auf und durchläuft einfach das Ergebnis. Noch einmal,MessageLayerDie API sollte bereits an anderer Stelle getestet werden. Dies lässt uns im Wesentlichen nichts zu testen. Es gibt keine direkt sichtbaren Nebenwirkungen des externen Codes, und wir sollten die interne Implementierung nicht testen. Das führt uns zu der einen Schlussfolgerung, dass es unangemessen wäre, diesen Code überhaupt zu testen . (Weitere Informationen zu dieser Argumentation finden Sie in Sandi Metz 'Präsentation Magic Tricks of Testing , [ Folien , Video ])

Frage 2: Warte, dann ... was? Ja, das stimmt, testen Sie das überhaupt nicht. Wie bereits erwähnt, handelt es sich hierbei um eine abgekürzte Version des Codes. Wenn Sie eine andere Logik haben, testen Sie diese, kapseln Sie diese jedoch in eine separate Einheit (wie die MessageSenderoben erwähnte Implementierung). Sie können dann den gesamten Aspekt des Codes leicht verspotten und gleichzeitig andere Logik testen.

Sie verwenden grundsätzlich eine Drittanbieter-API direkt in Ihrem Code. Code von Drittanbietern ist bekanntermaßen schwer zu testen, da er diese Art von Abhängigkeitsproblemen aufweisen kann, die Sie hier haben. Das Einkapseln in einen eingeklemmten Bereich kann das Testen Ihres anderen Codes vereinfachen und die Operation mit Schrotflinten reduzieren, wenn dieser Drittanbieter seinen Code ändert (oder nur ändert). Während das Testen des Teils, der mit der API eines Drittanbieters interagiert, immer noch Probleme bereitet, ist es auf eine kleine Facette beschränkt, die Sie isolieren können.

cbojar
quelle
+1 Stimme voll und ganz dem Konstruktor zu. Ich bin mir nicht sicher, ob die Rückgabe von NullMessage besser ist als die Rückgabe eines Mocks - es scheint, als würde ich aus technischen Gründen vorbeikommen. In jedem Fall ist das Ergebnis eine "Fälschung".
Rob
Lol Ich habe den Trick "Eigenen Builder als Argument nehmen" aus Guavas Quelle gestohlen.
Durron597
@RobY Ein Null-Objekt muss keine Fälschung sein, wenn Sie es als Teil Ihrer Anwendung als legitimes No-Op verwenden. Wenn es jedoch wirklich nur als Testfälschung verwendet wird, Messagesollte eine andere Art von Real verwendet werden.
Cbojar
@cbojar Zugegeben, aber Optional / Vielleicht ist dies ein besserer Weg, um nullfähige Referenzen zu verarbeiten. Wenn Sie nur ein NullObject für Unit-Tests schreiben, ist das NullObject wirklich nur ein handgeschriebenes Modell, wie früher. Das Problem ist, dass es ohne eine klare Begründung für die Regel "Keine Verspottungen von Verspottungen" schwer zu sagen ist, ob ein NullObject schlecht ist oder warum , außer dass es möglicherweise gegen eine subjektive Stilregel verstößt oder nicht.
Rob
Übrigens Messagehängt die einzige echte Implementierung von der API eines Drittanbieters ab. Um Instanzen ihrer Klassen erstellen zu können, müssen Sie ihre starten, Enginewas unter anderem einen Lizenzschlüssel erfordert ... nicht genau das, was Sie in einem Komponententest wollen.
Durron597
2

Ich werde @Robert Harvey zustimmen. Um ganz klar zu sein: Demeter ist vernünftig und hat einen guten Programmierstil. Es ist das "No Mocks from Mocks", das mir eher als subjektive Präferenz erscheint, die einen bestimmten Codierungsstil unterstützt, als als eine allgemein anwendbare (und gut begründete) Praxis. Die Feenregel entfernt "fließende" Schnittstellen wie:

Thing.create ("zoom"). SetDomain ("bo.com"). Add (1) .flip (). Reverse (). TuneForSemantics (). Run ();

Eine Art extremes Beispiel, aber im Wesentlichen würde die Feenregel es nicht zulassen, diese Klasse in einen Code aufzunehmen, da der Code nicht mehr testbar wäre. Aber das ist ein beliebtes Paradigma im OO-Code.

Das allgemeinere Problem besteht auch darin, wie eine Fabrik verspottet wird, um eine Verspottung zurückzugeben, mit der Sie testen möchten. Ich bin generell schüchtern, Fabriken als Abhängigkeiten zu verwenden, aber manchmal ist es viel besser als die Alternative. Wenn Sie mit enden

ThirdPartyThing ThirdPartyFactory<ThirdPartyThing>#create()

Ich sehe nicht, wie du es umgehen kannst. Sie benötigen einen Mock, um den Mock zurückzugeben. Diese Regel schlägt also zwei wirklich mächtige OO-Designmuster aus.

Ich kann mir keine Möglichkeit vorstellen, Ihr Problem zu umgehen, ohne diese Methode in 2 oder 3 aufzuteilen, lange Zeilen an den Client zu senden oder eine seltsame Stateful-Wrapper-Klasse zu erstellen.

Es würde mich wirklich interessieren, wie die Alternative aussieht.

Meine Antwort: Ihr Code ist in Ordnung! Excelcior!

(Eigentlich bin ich neugierig auf Alternativen)

...

Nehmen wir einen Grenzfall: Dürfen sich Mocks selbst zurückgeben? Technisch gesehen geben sie einen Schein zurück. Wenn nicht, dann schlägt das den GoF-Prototyp aus, und das ist eines der Muster, die sich im Laufe der Zeit gehalten haben:

MuActor prototype = ...
...
MuActor actor = prototype.create();
actor.run();

so erlaubt die Regel:

prototype = Mock(MuActor.class);
when(prototype.create()).thenReturn(prototype);

Außerdem verbietet die Feenregel die Verwendung von Monaden so ziemlich, da sie auf der Verkettung von Operationen für einen bestimmten Containertyp basieren. Sie könnten den Monadentyp testen, aber Sie könnten keinen Code testen, in dem die Monade erscheint.

rauben
quelle
Fließende Schnittstellen können Demeter weiterhin folgen, da fließende Schnittstellen dasselbe Objekt zurückgeben. Mit anderen Worten, obj.law().of().demeter();ist das gleiche wie obj.law(); obj.of(); obj.demeter();, was vollkommen akzeptabel ist. Eine ähnliche Erklärung kann für Prototypen gegeben werden.
Cbojar
Ja, aber wenn sie tatsächlich funktionieren und Sie sie verspotten möchten, stoßen Sie auf die Regel "Keine Verspottungen von Verspottungen". Dies bedeutet, dass der Code, der sie verwendet, nicht mehr testbar ist.
Rob
@bojar oh, und ich sollte klar sein. Demeter klingt vernünftig und hat einen guten Programmierstil. Es ist das "No Mocks from Mocks", das mir eher als subjektive Präferenz erscheint, die einen bestimmten Codierungsstil unterstützt, als als eine allgemein anwendbare (und gut begründete) Praxis.
Rob
Übrigens verwende ich in einem anderen Teil des Codes die fließende Schnittstelle. Ich habe ein paar Ideen aus dieser Blog - Post , und hier ist die eigentliche Code ich benutze: pastebin.com/D1GxSPsy
durron597
Das ist schlau. Cool! Ich werde es in dem Code versuchen, an dem ich gerade arbeite. Nur eine Anmerkung - eine fließende Benutzeroberfläche muss sich nicht selbst zurückgeben. Es gibt die Variante, bei der die fließende Schnittstelle eine Reihe unveränderlicher Instanzen zurückgibt. dh anstelle von "return this" wird "return new Fluent (...)" verwendet. Dies ist jedoch für den Kunden transparent, sodass es keinen Einfluss auf die Diskussion hier hat. Aber es ist eine lustige Tatsache.
Rob