Testowanie jednostkowe na przykładowym kodzie

0
TomRiddle napisał(a):

Myślenie życzeniowe i bajeczki.

Nie rozumiem. Jeśli nie ufam, że dostawca jakiejś zewnętrznej klasy/API/czegokolwiek ma dobre intencje to po prostu z tego nie korzystam.

TomRiddle napisał(a):

Jedyne co możesz zrobić to:

  1. Konsumować to co ktoś Ci dostarcza, ewentualnie dodając przypadki testowe na rzeczy które może Ci wrzucić
  2. Przetestować tylko Twoją część aplikacji.

Nic innego nie możesz zrobić. Jeśli robisz integrację w swojej aplikacji, i korzystasz z jakiegoś API, w tym momencie to się staję Twoją zależnością, i musisz to traktować jako koszt i coś czemu podlegasz. Łatwo jest sobie wmówić "on złamał kontrakt, więc to jest jego wina", ale to bajeczka.

Mogę zamodelować jakiś zewnętrzny komponent właśnie w postaci jakiegoś fake-obiektu/mocka. Model ten powinien w prosty sposób odwzorowywać zachowywanie realnego obiektu. Zakładam, że mój model dobrze odwzorowuje pierwowzór. Jeśli zmieni się kontrakt (zachowanie lub API), to niestety trzeba zmienić testowy model.

0
maszrum napisał(a):
TomRiddle napisał(a):

Myślenie życzeniowe i bajeczki.

Nie rozumiem. Jeśli nie ufam, że dostawca jakiejś zewnętrznej klasy/API/czegokolwiek ma dobre intencje to po prostu z tego nie korzystam.

TomRiddle napisał(a):

Jedyne co możesz zrobić to:

  1. Konsumować to co ktoś Ci dostarcza, ewentualnie dodając przypadki testowe na rzeczy które może Ci wrzucić
  2. Przetestować tylko Twoją część aplikacji.

Nic innego nie możesz zrobić. Jeśli robisz integrację w swojej aplikacji, i korzystasz z jakiegoś API, w tym momencie to się staję Twoją zależnością, i musisz to traktować jako koszt i coś czemu podlegasz. Łatwo jest sobie wmówić "on złamał kontrakt, więc to jest jego wina", ale to bajeczka.

Mogę zamodelować jakiś zewnętrzny komponent właśnie w postaci jakiegoś fake-obiektu/mocka. Model ten powinien w prosty sposób odwzorowywać zachowywanie realnego obiektu. Zakładam, że mój model dobrze odwzorowuje pierwowzór. Jeśli zmieni się kontrakt (zachowanie lub API), to niestety trzeba zmienić testowy model.

Skupiasz się na rzeczach które nie mają znaczenia, ale nie testujesz rzeczy które mają znaczenie.

Nie wiem czemu tak się trzymasz tego słowa "kontrakt", jakby to dawało Ci cokolwiek wartościowego.

Kontrakty są pomocne kiedy coś implementujesz, ale kiedy testujesz swoje rozwiązanie to zaciemniają tylko obraz, dają Ci fałszywe poczucie całości.

0
Afish napisał(a):

Heh, coś czuję, że już wiem, gdzie to zmierza. W sumie to ciekawe, że jest to forum programistyczne, ale w tematach ogólnych ludzie niemal nigdy nie pokażą kodu, za to bez problemu piszą, jakiego to oni kodu nie potrafią stworzyć.

Wymaganie jest proste: nie możesz użyć prawdziwego doWithOptionalBackupConnection, tylko musisz go wymockować. Napisałeś wcześniej, że to bankowo da się przetestować mockując Client, więc pokaż ten test i po sprawie.

Wspominałem już, że z takim wymaganiem testu jednostkowego nie napiszę. Z pisaniem testów odnosiłem się do normalnej sytuacji, w której mogę napisać test mockując zewnętrzne zależności. Kod wygląda wtedy tak:

using System;
using Xunit;
using NSubstitute;
using FluentAssertions;
using System.Threading.Tasks;

namespace TestProject1
{
    public class UnitTest1
    {
        [Fact]
        public void SyncDoWork_WhenFirstClientFails_ThenBackupClientIsUsed()
        {
            var mainClient = Substitute.For<Client>();
            mainClient.GetData(default).ReturnsForAnyArgs(_ => throw new InvalidOperationException());

            var backupClient = Substitute.For<Client>();
            backupClient.GetData(default).ReturnsForAnyArgs("dupa");

            new SyncWorker().DoWork(mainClient, backupClient).Should().Be("dupadupadupa");
        }

        [Fact]
        public void NonSyncDoWork_WhenFirstClientFails_ThenBackupClientIsUsed()
        {
            var mainClient = Substitute.For<NonSyncClient>();
            mainClient.GetData(default).ReturnsForAnyArgs<Task<string>>(_ => throw new InvalidOperationException());

            var backupClient = Substitute.For<NonSyncClient>();
            backupClient.GetData(default).ReturnsForAnyArgs("dupa");

            new NonSyncWorker().DoWork(mainClient, backupClient).Should().Be("dupadupadupa");
        }
    }

    class Util
    {
        public TResult DoWithOptionalBackupConnection<Client, TResult>(Client mainClient, Client backupClient, Func<Client, TResult> func)
        {
            try
            {
                return func(mainClient);
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex);
            }

            return func(backupClient);
        }
    }

    public interface Client
    {
        public string GetData(string parameter);
    }

    class SyncWorker
    {
        private readonly Util utilInstance = new();

        public string DoWork(Client mainClient, Client backupClient)
        {
            string first = utilInstance.DoWithOptionalBackupConnection(mainClient, backupClient, client => client.GetData("First"));
            string second = utilInstance.DoWithOptionalBackupConnection(mainClient, backupClient, client => client.GetData("Second"));
            string third = utilInstance.DoWithOptionalBackupConnection(mainClient, backupClient, client => client.GetData("Third"));
            return $"{first}{second}{third}";
        }
    }

    public interface NonSyncClient
    {
        public Task<string> GetData(string parameter);
    }

    class NonSyncWorker
    {
        private readonly Util utilInstance = new();

        public string DoWork(NonSyncClient mainClient, NonSyncClient backupClient)
        {
            Task<string> first = utilInstance.DoWithOptionalBackupConnection(mainClient, backupClient, client => client.GetData("First"));
            Task<string> second = utilInstance.DoWithOptionalBackupConnection(mainClient, backupClient, client => client.GetData("Second"));
            Task<string> third = utilInstance.DoWithOptionalBackupConnection(mainClient, backupClient, client => client.GetData("Third"));
            return $"{first.Result}{second.Result}{third.Result}";
        }
    }
}

Po zmianie klienta, kompilator będzie wymagał zmiany z 27 linijki.

Przy założeniu, że muszę stosować jakieś 3rd party, które zawiera logikę wykonującą mój kod, ale nie mogę ani go użyć ani zamockować jego zależności i przetestować normalnie, to najpierw proszę ownerów tego kodu o jego poprawienie, a jeśli się nie da, to po prostu nie piszę testu jednostkowego, muszą mi wystarczyć integracyjne.

Przykład @maszrum niczego niestety nie dowodzi, bo tam jest mock ze skopiowaną logiką produkcyjną. W rzeczywistym świecie to przestaje sensownie działać zazwyczaj wcześniej niż później. A jeśli nawet później, to jest jeszcze gorzej, bo nikt nie pamięta co tam zostało odwalone, więc i naprawienie potrwa dłużej niż powinno.

1
somekind napisał(a):

Przy założeniu, że muszę stosować jakieś 3rd party, które zawiera logikę wykonującą mój kod, ale nie mogę ani go użyć ani zamockować jego zależności i przetestować normalnie, to najpierw proszę ownerów tego kodu o jego poprawienie, a jeśli się nie da, to po prostu nie piszę testu jednostkowego, muszą mi wystarczyć integracyjne.

Nie muszą, właśnie o tym jest ten temat. Możesz napisać test jednostkowy sprawdzający, czy Twój argument spełnia kontrakt. Ten test wyglądałby tak:

public void test(){
   // Arrange
   PrepareClientForThrowingException();

   // Act & Assert
   assertThrows((client => client.GetData("blah"))(clientUnderTest));
}

Oczywiście ta lambda byłaby instancją tej lambdy z kodu produkcyjnego, a sam test byłby zapewne napisany czytelniej, ale chodzi o koncept.

Jesteś w stanie napisać ten test dla klienta synchronicznego, a przy zmianie klienta na asynchroniczny nie dasz rady tego poprawić. I to jest normalny test jednostkowy, sprawdzający dokładnie ten element aplikacji, o który nam chodzi, czyli kontrakt metody DoWithOptionalBackupConnection.

Przykład @maszrum ma tę wadę, że jest jedynie równoważny implementacji produkcyjnej, a nie równy, więc w przypadku dowolnej zmiany kodu produkcyjnego rozleci się.Twój przykład @somekind wykorzystuje implementację z zewnętrznej biblioteki, której po prostu nie chcemy użyć w teście.

0

@Afish

Tu nie chodzi o to, żeby sobie przetestować mocki na zielono, tylko żeby mieć test, który wywali się na czerwono, gdy zmieniliśmy klienta z synchronicznego na asynchronicznego

a tak?

var test = typeof(AsyncClientBad).GetMethods().First(x => x.Name == nameof(AsyncClientBad.Foo));
var isAsync = test.CustomAttributes.First().AttributeType.Name == nameof(AsyncStateMachineAttribute); 
Assert.False(isAsync);

// Z tym first i nazwą atrybutu oczywiście upraszczam, pewnie trzeba byłoby jeszcze kilka LoC
// dopisać aby mieć przyzwoity is async checker, ale to tylko pocka 

w kontekście tego twojego kodu który wrzucałeś na fiddla

public static void Async()
{
	var test = typeof(AsyncClientBad).GetMethods().First(x => x.Name == nameof(AsyncClientBad.Foo));
	var isAsync = test.CustomAttributes.First().AttributeType.Name == nameof(AsyncStateMachineAttribute);
	Console.WriteLine($"IsAsync? {isAsync}");

	if (isAsync)
	{
		throw new Exception("halt");
	}

	Task<String> a = Do<Task<String>, IAsyncClient>(new AsyncClientBad(), new AsyncClientGood(), client => client.Foo());
	Console.WriteLine(a.Result);
}

public static T Do<T, C>(C a, C b, Func<C, T> func)
{
	try
	{
		return func(a);
	}
	catch (Exception e)
	{
		Console.WriteLine(e);
	}

	return func(b);
}

returns

System.Exception: Bad
   at SyncClientBad.Foo()
   at Program.<>c.<Sync>b__1_0(ISyncClient client)
   at Program.Do[T,C](C a, C b, Func`2 func)
Good
IsAsync? True
Unhandled exception. System.Exception: halt
   at Program.Async()
   at Program.Main(String[] args)
   at Program.<Main>(String[] args)
Command terminated by signal 6

no generalnie chodzi mi o to, że mamy przecież do dyspozycji refleksję, kompilator, static code analysis, blabla, to może od tej strony warto dodatkowo podejść

0

@1a2b3c4d5e: Hmm, moim zdaniem słabe. Ten test znowu jest tylko równoważny implementacji w danej chwili i łatwo się z nią rozjedzie. Ponadto nie pokazuje jasno, dlaczego sprawdzasz akurat w taki sposób, musiałbyś dodać pewnie sporo komentarzy, żeby to było jakoś użyteczne.

Ktoś mógłby zrobić metodę zwracającą Task.Run() i Twój test tego nie wyłapie. A że async jest tylko w C#, to nie jest to takie nieprawdopodobne, mógłbyś użyć chociażby klienta napisanego w F# czy VB i wtedy byłby klops.

4

Zupełnie obok tematu. Jeśli rozważamy ten kod z punktu widzenia roku 2022 to mamy tam:
a) exception oriented programming
b) "gołe" Futures (sądząc po zachowaniu),
c) i te futures jeszcze z WaitForResult (czyli w javie get)

Masakra.

Co najmniej 2 ważne powody, żeby szukać jakiegoś alternatywnej implementacji doWithOptionalBackupConnection.
I do tego dyskwalifikacja "naszego" kodu w doWork.

To nie powinno trafić na produkcję nie z powodu testów, to powinno utknąć na review.

0

Jeśli dobrze rozumiem, to problem z asynchronicznym kodem miałby być taki, że nie dałoby się łapać wyjątków?
W JS się da. Jak się używa async/await to można robić tak samo try/catch, jak w synchronicznym.
Plus mając gołe promisy można wywołać metodę .catch do łapania wyjątków.

Ale może nie do końca zrozumiałem problem, jaki macie z tym, że coś jest asynchroniczne w kontekście łapania błędów.

0

@Afish

A że async jest tylko w C#, to nie jest to takie nieprawdopodobne, mógłbyś użyć chociażby klienta napisanego w F# czy VB i wtedy byłby klops.

Zwróć proszę tylko uwagę iż każde takie podniesienie poprzeczki sprawia że rozwiązanie (stosując w/w podejście) staje się mniej atrakcyjne

bo (wydaje mi się) że mógłbyś koniec końców z wygenerowanej dllki / exeka wyciągnąć to co tam ostatecznie jest i na tej podstawie odrzucić build na pipeline

ale no nie jest to zbyt ładne

0
Afish napisał(a):

Oczywiście ta lambda byłaby instancją tej lambdy z kodu produkcyjnego, a sam test byłby zapewne napisany czytelniej, ale chodzi o koncept.

Jesteś w stanie napisać ten test dla klienta synchronicznego, a przy zmianie klienta na asynchroniczny nie dasz rady tego poprawić. I to jest normalny test jednostkowy, sprawdzający dokładnie ten element aplikacji, o który nam chodzi, czyli kontrakt metody DoWithOptionalBackupConnection.

Przyznam, że ciekawy pomysł, ale ja bym takiego testu nawet nie tyle nie napisał, co go wywalił, gdybym coś takiego spotkał. Taki test nie ma wartości w momencie jego napisania, a poza tym wygląda jakby próbował zastępować kompilator.

Ale ja jestem skrzywiony, po pierwsze tym, że jak nie mogę napisać testu po swojemu, to wolę nie pisać wcale; a po drugie C#, i w takiej sytuacji zacząłbym szukać DoWithOptionalBackupConnectionAsync - brak takiej metody byłby mocnym wskazaniem, że trzeba przemyśleć jak się zachowa kod po takiej zmianie.

LukeJL napisał(a):

Jeśli dobrze rozumiem, to problem z asynchronicznym kodem miałby być taki, że nie dałoby się łapać wyjątków?
W JS się da. Jak się używa async/await to można robić tak samo try/catch, jak w synchronicznym.
Plus mając gołe promisy można wywołać metodę .catch do łapania wyjątków.

Ale może nie do końca zrozumiałem problem, jaki macie z tym, że coś jest asynchroniczne w kontekście łapania błędów.

@LukeJL: można łapać wyjątki w asynchronicznym kodzie. Tutaj problem polega na tym, że przekazujesz swój kod to jakiejś biblioteki 3rd party, której musisz użyć, a która nie używa poprawnie kodu asynchronicznego, więc możesz się nadziać na taki problem.

1 użytkowników online, w tym zalogowanych: 0, gości: 1