2010-04-12 13 views
10

Zacząłem używać AutoFixture http://autofixture.codeplex.com/, ponieważ moje testy jednostkowe były przepełnione wieloma ustawieniami danych. Spędzałem więcej czasu na ustawianiu danych niż na pisaniu testu jednostkowego. Oto przykład tego, jak mój pierwszy test jednostka wygląda (przykład pobranych z próbki aplikacji cargo z DDD blue book)Refaktoryzacja AutoFixture

[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(); 
} 

Oto jak próbowałem byłaby to 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))); 
} 

chciałbym wiedzieć, czy istnieje lepszy sposób na jego refaktoryzację. Chciałbym zrobić to krótko i łatwiej.

Odpowiedz

14

Twoja początkowa próba wygląda dobrze, ale jest co najmniej kilka rzeczy, które możesz nieco uprościć.

Przede wszystkim powinieneś być w stanie zmniejszyć w ten sposób:

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

do tego:

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

ponieważ nie używasz tych innych zmiennych. Jednak to zasadniczo blokuje wszelkie tworzenie CarrierMovement, aby użyć tych samych czterech wartości. Chociaż każdy utworzony CarrierMovement będzie osobną instancją, wszystkie będą miały te same cztery wartości i zastanawiam się, czy to właśnie miałeś na myśli?

Podobnie jak powyżej, zamiast

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

można napisać

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

ponieważ nie użyć zmiennej carrierM. Wpisanie typu wymyśli, że rejestrujesz Harmonogram ze względu na typ zwrotu Func.

Jednak zakładając, że konstruktor Harmonogram wygląda następująco:

public Schedule(IEnumerable<CarrierMovement> carrierMovements) 

można zamiast właśnie zarejestrowany carrierMovements tak:

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

co spowodowałoby AutoFixture automatycznie rozwiązać Harmonogram poprawnie. To podejście jest łatwiejsze w utrzymaniu, ponieważ pozwala na dodanie parametru do konstruktora harmonogramu w przyszłości bez przerwania testu (o ile AutoFixture może rozwiązać typ parametru).

Jednak w tym przypadku możemy zrobić coś lepszego, ponieważ tak naprawdę nie używamy zmiennej carrierMovements do celów innych niż rejestracja. To, co naprawdę musimy zrobić, to po prostu powiedzieć AutoFixture, jak stworzyć instancje IEnumerable<CarrierMovement>. Jeśli nie dbają o numerze 50 (nie powinien), możemy nawet użyć składni grupy metody takiego:

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

zauważyć brak metody nawiasach wywołania: Jesteśmy rejestracji Func, a ponieważ metoda zwraca IEnumerable<T> typowanie inferencyjne zajmuje się resztą.

Jednak to są wszystkie szczegóły.Na wyższym poziomie możesz rozważyć nierejestrowanie CarrierMovement w ogóle. Zakładając, że konstruktor ten:

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

autofiltry powinny być w stanie samemu to rozgryźć.

Utworzy on nową instancję Lokalizacji dla każdego miejsca odjazdu i przylotu, ale nie różni się od tego, co zrobiłeś ręcznie w oryginalnym teście.

Jeśli chodzi o czasy, domyślnie AutoFixture używa DateTime.Now, co zapewnia, że ​​czas przyjazdu nigdy nie będzie dłuższy niż czas odjazdu. Są jednak bardzo prawdopodobne, że są identyczne, ale zawsze można zarejestrować funkcję automatycznego zwiększania, jeśli jest to problem.

Biorąc pod uwagę powyższe rozważania, oto alternatywa:

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(); 
} 

Aby rozwiązać ten problem z IList<CarrierMovement> trzeba będzie go zarejestrować. Oto jeden ze sposobów, aby to zrobić:

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

Ponieważ jednak pytasz, to oznacza, że ​​konstruktor Harmonogram wygląda następująco:

public Schedule(IList<CarrierMovement> carrierMovements) 

i naprawdę myślę, że należy rozważyć zmianę tego interfejsu do podjęcia IEnumerable<Carriemovement>. Z perspektywy projektowania interfejsu API dostarczanie kolekcji przez dowolny element członkowski (w tym konstruktor) oznacza, że ​​członek może modyfikować kolekcję (na przykład poprzez wywoływanie jej metod Dodaj, Usuń i Wyczyść). Nie jest to zachowanie, którego można oczekiwać od konstruktora, więc nie pozwalaj na to.

AutoFixture automatycznie wygeneruje nowe wartości dla wszystkich obiektów Location w powyższym przykładzie, ale ze względu na szybkość procesora, kolejne wystąpienia DateTime będą prawdopodobnie identyczne.

Jeśli chcesz zwiększyć wartości DateTimes, możesz napisać małą klasę, która zwiększa wartość zwróconego obiektu DateTime przy każdym wywołaniu. Zostawię realizacji tej klasy do zainteresowanego czytelnika, ale można następnie zarejestrować go tak:

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

zakładając ten API (zawiadomienie raz bardziej składnia Metoda Grupa powyżej):

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

Dzięki za twoje komentarze. Jednak mam wyjątek zgłoszony przez AutoFixture Ploeh.AutoFixture.ObjectCreationException: AutoFixture nie był w stanie utworzyć wystąpienia typu System.Collections.Generic.IList'1 [DDDBookingApplication.Domain.Voyage.CarrierMovement], ponieważ nie ma publicznego konstruktor. Zakładam, że powinienem powiedzieć, jak utworzyć CarrierMovement? –

+0

Chciałbym również mieć inny zestaw danych dla wszystkich instats. Jakie są twoje myśli? –

+0

Dzięki za wszystkie szczegóły. Test jest teraz krótki i przechodzi :) –

Powiązane problemy