Testowanie jednostkowe na przykładowym kodzie

0
maszrum napisał(a):

Nie rozumiem tej części wypowiedzi. W sensie, że testować na prawdziwym kliencie? Czy coś innego?

Tak, na prawdziwym kliencie.

maszrum napisał(a):

Generalnie zadanie było takie, żeby napisać testy które wykażą, że zmiana interfejsu Client na wersję asynchroniczną nie jest "kompatybilna" z operacją w jakiejś zewnętrznej bibliotece. I wydaje mi się, że napisałem kod który właśnie takie coś robi. Przynajmniej tak mi się wydaje.

Zgadza się, Twój kod robi to, co potrzeba. Teraz pozostają jedynie rozważania, czy Twój test jest dobrej jakości i tego tyczył się mój poprzedni komentarz.

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

Sensowniejszym w mojej opinii jest test, który sprawdza kontrakt explicite. Czyli po prostu sprawdza, że klient rzuca wyjątek, a nie robi mockowego klienta z mockową implementacją zależności.

Nie rozumiem tej części wypowiedzi. W sensie, że testować na prawdziwym kliencie? Czy coś innego?

Generalnie zadanie było takie, żeby napisać testy które wykażą, że zmiana interfejsu Client na wersję asynchroniczną nie jest "kompatybilna" z operacją w jakiejś zewnętrznej bibliotece. I udało mi się napisać kod który właśnie takie coś robi. Przynajmniej tak mi się wydaje.

Ale Twój test od początku był kulawy bo nie testował rzucenia wyjątku w pierwszej wersji, tej synchronicznej.

0
maszrum napisał(a):

Nie no, miałem taki test-case od samego początku tutaj.

Noi co ten test testuje, konkretnie?

Assert.That(mainClient.GetDataCalledTimes, Is.EqualTo(3));
Assert.That(backupClient.GetDataCalledTimes, Is.EqualTo(3));

To nie jest dobry test. Dla mnie - równoważne z brakiem testu.

Mówiąc to...

TomRiddle napisał(a):

Ale Twój test od początku był kulawy bo nie testował rzucenia wyjątku w pierwszej wersji, tej synchronicznej.

Mówiąc to, nie miałem na myśli "test w którym jest rzucenie wyjątki", tylko "test który testuje to co ma się stać kiedy poleci wyjątek". Dwa różne koncepty. U Ciebie jest ten pierwszy test, i dlatego nie wychwycił tego co się stało jak zmieniłeś implementację na asynchroniczną.

0
TomRiddle napisał(a):

Noi co ten test testuje, konkretnie?

Assert.That(mainClient.GetDataCalledTimes, Is.EqualTo(3));
Assert.That(backupClient.GetDataCalledTimes, Is.EqualTo(3));

To nie jest dobry test. Dla mnie - równoważne z brakiem testu.

Test ten sprawdza:

  • czy metoda GetData dla klienta głównego została wykonana 3 razy,
  • czy metoda GetData dla klienta zapasowego została wykonana 3 razy.

Dlaczego to nie jest dobry test? W jaki sposób powinien wyglądać ten kod, aby w ogóle można było go nazwać testem?

Mówiąc to...

TomRiddle napisał(a):

Ale Twój test od początku był kulawy bo nie testował rzucenia wyjątku w pierwszej wersji, tej synchronicznej.

Mówiąc to, nie miałem na myśli "test w którym jest rzucenie wyjątki", tylko "test który testuje to co ma się stać kiedy poleci wyjątek". Dwa różne koncepty. U Ciebie jest ten pierwszy test, i dlatego nie wychwycił tego co się stało jak zmieniłeś implementację na asynchroniczną.

Tej części niestety nie rozumiem (jak napisałem w komentarzu), więc na razie nie będę się do niej odnosił.

0

@maszrum: Twoje testy mają jeszcze jedną wadę. Jeżeli ja teraz poprawię produkcyjnego klienta tak, żeby używał pod spodem synchronicznego, czyli zrobię coś takiego:

class AsyncClient {
    public Task<string> GetData(string parameter) {
        var data = syncClient.GetData(parameter);
        return Task.FromResult(data);
    }
}

Teraz Twoje testy dalej nie przechodzą, bo operujesz na mockach, a jednak kod produkcyjny działa (przynajmniej na oko powinien działać, nie uruchamiałem go). Czyli teraz nie tylko muszę naprawić produkcyjny kod, ale też testy, czyli mam spory coupling, a tego nie chcę.

Oczywiście taki kod produkcyjny nie ma sensu, ale to tylko przykład. Bardziej realny mógłby być taki:

class AsyncClient {
    public Task<string> GetData(string parameter) {
        if(!CanGetData()){
            throw new Exception("Boo");
        }
        return Task.Run(() => GetDataInternal());
    }
}

I to znowu zadziała na produkcji, a nie zadziała z Twoimi testami.

0
Afish napisał(a):

I to znowu zadziała na produkcji, a nie zadziała z Twoimi testami.

No dobra, tylko dlaczego zakładasz, że kod, który nie przechodzi testów trafi na produkcję? :)

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

Noi co ten test testuje, konkretnie?

Assert.That(mainClient.GetDataCalledTimes, Is.EqualTo(3));
Assert.That(backupClient.GetDataCalledTimes, Is.EqualTo(3));

To nie jest dobry test. Dla mnie - równoważne z brakiem testu.

Test ten sprawdza:

  • czy metoda GetData dla klienta głównego została wykonana 3 razy,
  • czy metoda GetData dla klienta zapasowego została wykonana 3 razy.

Dlaczego to nie jest dobry test? W jaki sposób powinien wyglądać ten kod, aby w ogóle można było go nazwać testem?

Bo samo wywołanie metody nie jest faktycznym requirementem. Nie testujesz zwracanych wyników.

0
TomRiddle napisał(a):

Bo samo wywołanie metody nie jest faktycznym requirementem. Nie testujesz zwracanych wyników.

Moim celem nie było przetestowanie całej funkcjonalności tylko zrobienie takiego małego Proof of Concept. Jakbym miał testować całą funkcjonalność, to zrobiłbym osobne use-casy dla innych asercji.

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

Bo samo wywołanie metody nie jest faktycznym requirementem. Nie testujesz zwracanych wyników.

Moim celem nie było przetestowanie całej funkcjonalności tylko zrobienie takiego małego Proof of Concept. Jakbym miał testować całą funkcjonalność, to zrobiłbym osobne use-casy dla innych asercji.

Chodzi mi tylko o to, że te asercje:

Assert.That(mainClient.GetDataCalledTimes, Is.EqualTo(3));
Assert.That(backupClient.GetDataCalledTimes, Is.EqualTo(3));

są warte bardzo mało.

Gdybyś je usunął, to test miałby podobną wartość. Powinieneś jakąś bardziej ogarniętną asercję.

0
TomRiddle napisał(a):

Ale wiecie że kontraktów się nie da przetestować automatycznie, prawda? :D Jedynie zachowanie, które może być zgodne z kontraktem.

https://softwareskill.pl/consumer-driven-contract

Trochę się da, pytanie raczej kto ma ten test napisać, bo zwykle każdy testuje swój kawałek, a to co po środku leży odłogiem i może wyjdzie (a zwykle nie) w trakcie integracji.

1
piotrpo napisał(a):
TomRiddle napisał(a):

Ale wiecie że kontraktów się nie da przetestować automatycznie, prawda? :D Jedynie zachowanie, które może być zgodne z kontraktem.

https://softwareskill.pl/consumer-driven-contract

Trochę się da, pytanie raczej kto ma ten test napisać, bo zwykle każdy testuje swój kawałek, a to co po środku leży odłogiem i może wyjdzie (a zwykle nie) w trakcie integracji.

Możesz przetestować czy jakieś zachowanie wpasuje się w jakiś element kontraktu, ale samego kontraktu nie możesz przetestować.

  • Kontrakt to jest coś w stylu: "Math.sqrt(int) dla argumentu większego od 1, zawsze zwróci liczbę mniejszą niż argument". Czy możesz przetestować taki kontrakt? Nie

Owszem, możesz przetestować Math.sqrt(4) == 2, Math.abs(9) == 3, i więcej liczb, możesz przetestować ile liczb chcesz, ale to są zachowania. Taki input da mi taki output. Ale nie kontrakt.

Kontraktów nie da się przetestować.

4

@TomRiddle: Nie wiem, czy to co piszesz ma sens, którego nie rozumiem, czy bawisz się jedynie językiem.
Ogólnie testy (w tym jednostkowe) testują zachowanie, dlatego UT pisze się razem z kodem i mając:

int abs(int a){
  return a<0?0-a:a;
}

Określa się przypadki brzegowe, dajmy na to1, -1, 0 i testuje, wierząc, że nikt nie będzie tak durny, żeby wstawić tam linijkę if a==14 return 27;

Czyli moim zdaniem masz rację, ale jest to dyskusja raczej akademicka.

0
maszrum napisał(a):

No dobra, tylko dlaczego zakładasz, że kod, który nie przechodzi testów trafi na produkcję? :)

Bo klienta pisze jeden zespół, a używa go inny, na przykład. Test nie powinien sprawdzać szczególnego przypadku, tylko kontrakt, to wszystko. Jak kontrakt jest spełniony, to test ma przechodzić 🙂

0
Afish napisał(a):

Bo klienta pisze jeden zespół, a używa go inny, na przykład. Test nie powinien sprawdzać szczególnego przypadku, tylko kontrakt, to wszystko. Jak kontrakt jest spełniony, to test ma przechodzić 🙂

Nie wiem, mnie jakoś te argumenty nie przekonują. Napisałem test metody doWork, w taki sposób, że jestem w stanie wykryć niepoprawne użycie jakiegoś zewnętrznego komponentu, czy to za pomocą realnego obiektu, czy za pomocą mocka udającego prawdziwą funkcjonalność. Z całym szacunkiem, ale jakieś bajki o tym, że nieprzetestowany kod może trafić na produkcję jakoś mnie nie przekonują. A przypadek, który przytoczyłeś, w którym ktoś w metodzie asynchronicznej wrzucił kod synchroniczny też nie jest dla mnie argumentem (bo wtedy po co w ogóle te kombinacje z przerabianiem na async?). Jeśli test nie przechodzi, to niestety, ale kod (jakiejś funkcjonalności, nie testów) w miarę możliwości trzeba przerobić tak aby wszystkie testy przeszły. Ostatecznie można iść do osoby/firmy odpowiedzialnej za kod third party, aby dostosował go do naszych wymagań.

1
maszrum napisał(a):

Nie wiem, mnie jakoś te argumenty nie przekonują. Napisałem test metody doWork, w taki sposób, że jestem w stanie wykryć niepoprawne użycie jakiegoś zewnętrznego komponentu, czy to za pomocą realnego obiektu

No właśnie nie. Twój test sprawdza mocki, które w tym momencie są równoważne produkcyjnej implementacji, ale jutro już mogą nie być.

1
Afish napisał(a):

No właśnie nie. Twój test sprawdza mocki, które w tym momencie są równoważne produkcyjnej implementacji, ale jutro już mogą nie być.

Jak ktoś wystawia kontrakt, a potem go zmienia, to trochę słabo. To jest taka sama sytuacja, jakby ktoś zrobił jakieś breaking change w API i tym samym złamał kontrakt, który sam stworzył.

Edit: Albo ktoś podpisał kontrakt za 10 baniek, a potem jednostronnie zleceniodawca zmienił na 5 :).

0
maszrum napisał(a):

Jak ktoś wystawia kontrakt, a potem go zmienia, to trochę słabo. To jest taka sama sytuacja, jakby ktoś zrobił jakieś breaking change w API i tym samym złamał kontrakt, który sam stworzył.

Tak, pełna zgoda, w tym konkretnym przypadku to nie ma sensu, ale chodzi o ogólne podejście. Twój kod testuje mocki, to jest ogólnie złe, a czy w tym konkretnym przypadku to się zepsuje, czy może nie, to już osobna dyskusja.

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

No właśnie nie. Twój test sprawdza mocki, które w tym momencie są równoważne produkcyjnej implementacji, ale jutro już mogą nie być.

Jak ktoś wystawia kontrakt, a potem go zmienia, to trochę słabo. To jest taka sama sytuacja, jakby ktoś zrobił jakieś breaking change w API i tym samym złamał kontrakt, który sam stworzył.

Noooo, ale ameryki nie odkryłeś. Wiadomo że breaking changes są średnie.

Ale to nie znaczy że Twoje testy mogą tego nie testować. Bo to co mówisz to: "Breaking changes są średnie, ergo nie staną się, ergo nie muszą tego testować, ergo moje testy mogą być słabe".

0
TomRiddle napisał(a):

Noooo, ale ameryki nie odkryłeś. Wiadomo że breaking changes są średnie.

Ale to nie znaczy że Twoje testy mogą tego nie testować. Bo to co mówisz to: "Breaking changes są średnie, ergo nie staną się, ergo nie muszą tego testować, ergo moje testy mogą być słabe".

Jeśli ktoś mi wystawi jakiś kontrakt to powiedzmy, że mogę mu zaufać. Co więcej, w pewnym sensie muszę mu zaufać, bo inaczej nie mogę użyć jego rzeczy w moim kodzie. Żadne testy mi nie zagwarantują tego, czy on tam w pewnym momencie nie wstawi jakiś głupot, typu formatowanie dysku C: (albo cokolwiek innego). Oczekiwałbym, że jeśli zamierza wprowadzić jakieś zmiany w kontrakcie (API, czy czymkolwiek innym) to mnie wcześniej poinformuje, żebym miał czas na dostosowanie mojego kodu do tych zmian. Na przykład zmiana mocka, aby lepiej modelował nową wersję klasy third party.

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

Noooo, ale ameryki nie odkryłeś. Wiadomo że breaking changes są średnie.

Ale to nie znaczy że Twoje testy mogą tego nie testować. Bo to co mówisz to: "Breaking changes są średnie, ergo nie staną się, ergo nie muszą tego testować, ergo moje testy mogą być słabe".

Jeśli ktoś mi wystawi jakiś kontrakt to powiedzmy, że mogę mu zaufać. Co więcej, w pewnym sensie muszę mu zaufać, bo inaczej nie mogę użyć jego rzeczy w moim kodzie.

Myślenie życzeniowe i bajeczki.

Żadne testy mi nie zagwarantują tego, czy on tam w pewnym momencie nie wstawi jakiś głupot, typu formatowanie dysku C: (albo cokolwiek innego).

Noi zgadza się.

Oczekiwałbym, że jeśli zamierza wprowadzić jakieś zmiany w kontrakcie (API, czy czymkolwiek innym) to mnie wcześniej poinformuje, żebym miał czas na dostosowanie mojego kodu do tych zmian. Na przykład zmiana mocka, aby lepiej modelował nową wersję klasy third party.

Myślenie życzeniowe i bajeczki znowu.

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.

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