2010-04-12 3 views
10

j'ai commencé à utiliser AutoFixture http://autofixture.codeplex.com/ que mes tests unitaires était pléthorique avec beaucoup de configuration des données. Je passais plus de temps à configurer les données qu'à écrire mon test unitaire. Voici un exemple de la façon dont mon test unitaire initial ressemble (exemple tiré de l'échantillon de l'application de la cargaison de DDD livre bleu)AutoFixture refactorisation

[Test] 
public void should_create_instance_with_correct_ctor_parameters() 
{ 
    var carrierMovements = new List<CarrierMovement>(); 

    var deparureUnLocode1 = new UnLocode("AB44D"); 
    var departureLocation1 = new Location(deparureUnLocode1, "HAMBOURG"); 
    var arrivalUnLocode1 = new UnLocode("XX44D"); 
    var arrivalLocation1 = new Location(arrivalUnLocode1, "TUNIS"); 
    var departureDate1 = new DateTime(2010, 3, 15); 
    var arrivalDate1 = new DateTime(2010, 5, 12); 

    var carrierMovement1 = new CarrierMovement(departureLocation1, arrivalLocation1, departureDate1, arrivalDate1); 

    var deparureUnLocode2 = new UnLocode("CXRET"); 
    var departureLocation2 = new Location(deparureUnLocode2, "GDANSK"); 
    var arrivalUnLocode2 = new UnLocode("ZEZD4"); 
    var arrivalLocation2 = new Location(arrivalUnLocode2, "LE HAVRE"); 
    var departureDate2 = new DateTime(2010, 3, 18); 
    var arrivalDate2 = new DateTime(2010, 3, 31); 

    var carrierMovement2 = new CarrierMovement(departureLocation2, arrivalLocation2, departureDate2, arrivalDate2); 

    carrierMovements.Add(carrierMovement1); 
    carrierMovements.Add(carrierMovement2); 

    new Schedule(carrierMovements).ShouldNotBeNull(); 
} 

Voici comment j'ai essayé de le factoriser avec AutoFixture

[Test] 
public void should_create_instance_with_correct_ctor_parameters_AutoFixture() 
{ 
    var fixture = new Fixture(); 

    fixture.Register(() => new UnLocode(UnLocodeString())); 

    var departureLoc = fixture.CreateAnonymous<Location>(); 
    var arrivalLoc = fixture.CreateAnonymous<Location>(); 
    var departureDateTime = fixture.CreateAnonymous<DateTime>(); 
    var arrivalDateTime = fixture.CreateAnonymous<DateTime>(); 

    fixture.Register<Location, Location, DateTime, DateTime, CarrierMovement>(
     (departure, arrival, departureTime, arrivalTime) => new CarrierMovement(departureLoc, arrivalLoc, departureDateTime, arrivalDateTime)); 

    var carrierMovements = fixture.CreateMany<CarrierMovement>(50).ToList(); 

    fixture.Register<List<CarrierMovement>, Schedule>((carrierM) => new Schedule(carrierMovements)); 

    var schedule = fixture.CreateAnonymous<Schedule>(); 

    schedule.ShouldNotBeNull(); 
} 

private static string UnLocodeString() 
{ 
    var stringBuilder = new StringBuilder(); 

    for (int i = 0; i < 5; i++) 
     stringBuilder.Append(GetRandomUpperCaseCharacter(i)); 

    return stringBuilder.ToString(); 
} 

private static char GetRandomUpperCaseCharacter(int seed) 
{ 
    return ((char)((short)'A' + new Random(seed).Next(26))); 
} 

Je voudrais savoir s'il y a une meilleure façon de le refactoriser. Voudrait le faire plus court et plus facile que cela.

Répondre

14

Votre première tentative semble bon, mais il y a au moins deux choses que vous pouvez simplifier un peu.

Tout d'abord, vous devriez être en mesure de réduire ceci:

fixture.Register<Location, Location, DateTime, DateTime, CarrierMovement>(
    (departure, arrival, departureTime, arrivalTime) => 
     new CarrierMovement(departureLoc, arrivalLoc, departureDateTime, arrivalDateTime)); 

à ceci:

fixture.Register<Location, Location, DateTime, DateTime, CarrierMovement>(
    () => new CarrierMovement(departureLoc, arrivalLoc, departureDateTime, arrivalDateTime)); 

puisque vous n'êtes pas d'utiliser ces autres variables. Cependant, cela verrouille essentiellement toute création de CarrierMovement pour utiliser les mêmes quatre valeurs. Bien que chaque CarrierMovement créé sera une instance distincte, ils partageront tous les mêmes quatre valeurs, et je me demande si c'est ce que vous vouliez dire?

Dans la même veine que ci-dessus, au lieu de

fixture.Register<List<CarrierMovement>, Schedule>((carrierM) => 
    new Schedule(carrierMovements)); 

vous pouvez écrire

fixture.Register(() => new Schedule(carrierMovements)); 

puisque vous n'utilisez pas la variable carrierM. L'inférence de type comprendra que vous enregistrez un Schedule en raison du type de retour du Func.

Cependant, en supposant que le constructeur de l'annexe ressemble à ceci: vous pourriez plutôt

public Schedule(IEnumerable<CarrierMovement> carrierMovements) 

venez enregistré le carrierMovements comme ceci:

fixture.Register<IEnumerable<CarrierMovement>>(carrierMovements); 

qui causerait AutoFixture pour résoudre automatiquement Schedule correctement. Cette approche est plus maintenable car il vous permet d'ajouter un paramètre au constructeur de l'annexe à l'avenir sans casser le test (aussi longtemps que AutoFixture peut résoudre le type de paramètre).

Cependant, nous pouvons faire mieux que cela dans ce cas parce que nous ne avons pas vraiment utilisé la variable carrierMovements pour rien d'autre que l'inscription. Ce que nous devons vraiment faire, c'est simplement de dire à AutoFixture comment créer des instances de IEnumerable<CarrierMovement>. Si vous ne se soucient pas du nombre 50 (vous ne devriez pas), on peut même utiliser la syntaxe du groupe Méthode comme ceci:

fixture.Register(fixture.CreateMany<CarrierMovement>); 

Notez l'absence d'invocation méthode paranthèses: nous l'enregistrement d'un Func et car la méthode CreateMany<T> retourne IEnumerable<T> de type prend inférences charge du reste.

Cependant, ce sont tous les détails.À un niveau supérieur, vous pourriez envisager de ne pas enregistrer CarrierMovement du tout. En supposant que ce constructeur:

public CarrierMovement(Location departureLocation, 
    Location arrivalLocation, 
    DateTime departureTime, 
    DateTime arrivalTime) 

autofixture devrait être en mesure de le comprendre par lui-même.

Il va créer une nouvelle instance de Location pour chaque emplacement de départ et d'arrivée, mais ce n'est pas différent de ce que vous avez fait manuellement dans le test d'origine. AutoFixture utilise par défaut DateTime.Now, ce qui garantit au moins que l'heure d'arrivée ne sera jamais antérieure à l'heure de départ. Cependant, ils sont très susceptibles d'être identiques, mais vous pouvez toujours enregistrer une fonction auto-incrémentée si c'est un problème.

Eu égard à ces considérations, voici une alternative:

public void should_create_instance_with_correct_ctor_parameters_AutoFixture() 
{ 
    var fixture = new Fixture(); 

    fixture.Register(() => new UnLocode(UnLocodeString())); 

    fixture.Register(fixture.CreateMany<CarrierMovement>); 

    var schedule = fixture.CreateAnonymous<Schedule>(); 

    schedule.ShouldNotBeNull(); 
} 

Pour résoudre le problème avec IList<CarrierMovement> vous devez l'enregistrer. Voici une façon de le faire:

fixture.Register<IList<CarrierMovement>>(() => 
    fixture.CreateMany<CarrierMovement>().ToList()); 

Cependant, puisque vous me demandez, je veux dire que le constructeur de l'annexe ressemble à ceci:

public Schedule(IList<CarrierMovement> carrierMovements) 

et je pense vraiment que vous devriez revoir changer cette API pour prendre une IEnumerable<Carriemovement>. Du point de vue de la conception de l'API, fournir une collection à travers n'importe quel membre (y compris un constructeur) implique que le membre est autorisé à modifier la collection (par exemple en appelant ses méthodes Add, Remove et Clear). C'est à peine le comportement que vous attendez d'un constructeur, alors ne le permettez pas. AutoFixture génère automatiquement de nouvelles valeurs pour tous les objets Location dans mon exemple ci-dessus, mais en raison de la vitesse de la CPU, les instances suivantes de DateTime sont susceptibles d'être identiques.

Si vous souhaitez augmenter DateTimes, vous pouvez écrire une petite classe qui incrémente le DateTime renvoyé chaque fois qu'il est appelé. Je vais laisser la mise en œuvre de cette classe au lecteur intéressé, mais vous pouvez alors l'enregistrer comme ceci:

var dtg = new DateTimeGenerator(); 
fixture.Register(dtg.Next); 

en supposant cette API (avis une fois de plus la syntaxe du groupe méthode ci-dessus):

public class DateTimeGenerator 
{ 
    public DateTime Next(); 
} 
+0

Merci pour vos commentaires. Cependant, j'ai une petite exception levée par AutoFixture Ploeh.AutoFixture.ObjectCreationException: AutoFixture n'a pas pu créer une instance de type System.Collections.Generic.IList'1 [DDDBookingApplication.Domain.Voyage.CarrierMovement], car il n'a pas de public constructeur. Je suppose que je devrais dire comment créer CarrierMovement? –

+0

Je voudrais aussi avoir un ensemble de données différent pour toutes les instances. Quelles sont tes pensées? –

+0

Merci pour tous les détails. Le test est court maintenant et passe :) –