Rozmiar testów przebija rozmiar kodu?

Postanowiłem wreszcie nauczyć się tych nieszczęsnych testów...

Ćwiczebny problem, który mam nadzieję rozwinąć do czegoś sensownego i większego: Chatroom.

Krok pierwszy: Chatbox siedzący tylko w przeglądarce, user wpisuje swój nick i wiadomość i wiadomości pojawiają się jedna po drugiej. Nic użytecznego, ale już można dopisać testy.

Plik z takim minimalistycznym chatboxem: 40 linii.

Z kolei plik z testami do powyższego: 158 linii!

A to i tak nie wszystko, bo nie testuję jeszcze np. górnego ograniczenia na długość nicku i górnego ograniczenia na długość postu (ogarnąłem się, że są z tym pewne subtelności, których nie zauważyłem dotychczas - zamierzam się tym zająć wtedy, kiedy dodam zapisywanie wiadomości do bazy danych na serwerze(*))

Czy taka sytacja jest OK?Wydaje mi się tu być coś dziwnego. Z drugiej strony każdy test testuje jakąś założoną funkcjonalność, więc jakby żaden nie jest nadprogramowy?

To prawda, jest spora duplikacja kodu w testach. Możnaby ją usunąć np. w ten sposób, by wprowadzić pomocnicze metody PostMessage czy AssertMessageEqual. Ale nie zrobiłem tego świadomie, dlatego, że gdzieś wyczytałem, że do testów odnoszą się zupełnie inne zasady, niż do "normalnego" kodu: testy winny być w największym możliwym stopniu pozbawione logiki i napisane najprościej, jak to możliwe, by można było łatwo zobaczyć powód, dlaczego dany test nie przechodzi patrząc tylko na jego kod (a nie jeszcze na kod iluśtam pomocniczych metod i klas) i by nie musieć pisać testów testujących testy.

Ponieważ testy zawsze były moją piętą achillesową, czy mogę się zapytać, czy poniższe jest OK, czy jednak z braku wiedzy/doświadczenia robię coś bardzo złego i tego nie widzę?

(*) Idea ograniczenia [StringLength(maximumLength: 16)] na długość nicku jest taka, by później można było to wrzucić do bazy do kolumny typu CHAR[16]. Ale to tak nie działa, bo CHAR[16] to jest ilość bajtów w UTF-8, zaś [StringLength] wymusza ograniczenie na ilość znaków, a tak naprawdę to chyba na ilość 16-bitowych charów w kodowaniu UTF-16. Więc tak to nie zadziała. Chyba będę musiał przeliczyć na kartce, ile bajtów UTF-8 może w pesymistycznym przypadku zająć string o długości 16 dwubajtowych charów i testować, czy username wprowadzony do chatboxa po konwersji do UTF-8 może przebić tę wartość. Może więc jednak źle robię, bo zostawiłem nieotestowany kod, o którym wiem, że będzie musiał się zmienić?

Plik App.razor:

<EditForm Model="@pendingMessage" OnValidSubmit="Post">
    <label>Chose your name: <InputText class="username-input" @bind-Value="pendingMessage.UserName"/></label>

    <ol>
        @foreach(Message message in messages)
        {
            <li class="chat-message">
                <b class="chat-message-author">@message.UserName</b>: <span class="chat-message-content">@message.Content</span>
            </li>
        }
    </ol>

    <label>Type your message: <InputText class="chat-message-content-input" @bind-Value="pendingMessage.Content"/></label> <button class="post-chat-message-button" type="submit">Send</button>

    <DataAnnotationsValidator />
    <ValidationSummary />

</EditForm>

@code{
    Message pendingMessage = new();
    List<Message> messages = new();

    void Post()
    {
        messages.Add(new Message { UserName = pendingMessage.UserName, Content = pendingMessage.Content });
        pendingMessage.Content = "";
    }

    class Message
    {
        [Required]
        [StringLength(maximumLength: 16, MinimumLength = 1)]
        public string? UserName { get; set; }

        [Required]
        [StringLength(maximumLength: 160, MinimumLength = 1)]
        public string? Content { get; set; }
    }
}

I plik AppTests.razor:

@inherits TestContext

@code {
    [Fact]
    public void CanAddAMessage()
    {
        IRenderedFragment cut = RenderComponent<App>();
        IElement usernameInput = cut.Find(".username-input");
        IElement contentInput = cut.Find(".chat-message-content-input");
        IElement sendButton = cut.Find(".post-chat-message-button");

        string expectedUserName = "Master Cock";
        string expectedMessageContent = "You may be a treasure to men that prize you, but for me I would rather have a single barley-corn than a peck of pearls.";

        usernameInput.Change(expectedUserName);
        contentInput.Change(expectedMessageContent);
        sendButton.Click();

        string actualUserName = cut.Find(".chat-message-author").TextContent;
        string actualMessageContent = cut.Find(".chat-message-content").TextContent;

        Assert.Equal(expectedUserName, actualUserName);
        Assert.Equal(expectedMessageContent, actualMessageContent);
    }

    [Fact]
    public void ChatIsEmptyInTheBeginning()
    {
        IRenderedFragment cut = RenderComponent<App>();

        Assert.Empty(cut.FindAll(".chat-message"));
    }

    [Fact]
    public void AddingAMessageAddsOnlyOneMessage()
    {
        IRenderedFragment cut = RenderComponent<App>();
        IElement usernameInput = cut.Find(".username-input");
        IElement contentInput = cut.Find(".chat-message-content-input");
        IElement sendButton = cut.Find(".post-chat-message-button");

        string expectedUserName = "Master Cock";
        string expectedMessageContent = "Ho! ho! that's for me";

        usernameInput.Change(expectedUserName);
        contentInput.Change(expectedMessageContent);
        sendButton.Click();

        Assert.Single(cut.FindAll(".chat-message"));
    }

    [Fact]
    public void SecondMessageAppendsToTheFirstOne()
    {
        IRenderedFragment cut = RenderComponent<App>();
        IElement usernameInput = cut.Find(".username-input");
        IElement contentInput = cut.Find(".chat-message-content-input");
        IElement sendButton = cut.Find(".post-chat-message-button");

        string firstExpectedUserName = "Wolf";
        string firstExpectedMessageContent = "How dare you muddle the water from which I am drinking.";

        usernameInput.Change(firstExpectedUserName);
        contentInput.Change(firstExpectedMessageContent);
        sendButton.Click();

        string secondExpectedUserName = "Lambikin";
        string secondExpectedMessageContent = "Nay, master, nay, if the water be muddy up there, I cannot be the cause of it, for it runs down from you to me.";

        usernameInput.Change(secondExpectedUserName);
        contentInput.Change(secondExpectedMessageContent);
        sendButton.Click();

        string firstActualUserName = cut.FindAll(".chat-message-author")[0].TextContent;
        string firstActualMessageContent = cut.FindAll(".chat-message-content")[0].TextContent;
        string secondActualUserName = cut.FindAll(".chat-message-author")[1].TextContent;
        string secondActualMessageContent = cut.FindAll(".chat-message-content")[1].TextContent;

        Assert.Equal(firstExpectedUserName, firstActualUserName);
        Assert.Equal(firstExpectedMessageContent, firstActualMessageContent);
        Assert.Equal(secondExpectedUserName, secondActualUserName);
        Assert.Equal(secondExpectedMessageContent, secondActualMessageContent);
    }

    [Fact]
    public void PostingMessageClearsNextMessageContent()
    {
        IRenderedFragment cut = RenderComponent<App>();
        IElement usernameInput = cut.Find(".username-input");
        IElement contentInput = cut.Find(".chat-message-content-input");
        IElement sendButton = cut.Find(".post-chat-message-button");

        string userName = "Wolf";
        string messageContent = "There's my supper, if only I can find some excuse to seize it.";

        usernameInput.Change(userName);
        contentInput.Change(messageContent);
        sendButton.Click();

        Assert.Equal("", contentInput.GetAttribute("value"));
    }

    [Fact]
    public void PostingMessageDoesNotChangeNextMessageUsername()
    {
        IRenderedFragment cut = RenderComponent<App>();
        IElement usernameInput = cut.Find(".username-input");
        IElement contentInput = cut.Find(".chat-message-content-input");
        IElement sendButton = cut.Find(".post-chat-message-button");

        string expectedUserName = "Wolf";
        string messageContent = "I don't care, if it was not you, it was your father";

        usernameInput.Change(expectedUserName);
        contentInput.Change(messageContent);
        sendButton.Click();

        Assert.Equal(expectedUserName, usernameInput.GetAttribute("value"));
    }

    [Fact]
    public void CannotPostEmptyMessages()
    {
        IRenderedFragment cut = RenderComponent<App>();
        IElement usernameInput = cut.Find(".username-input");
        IElement contentInput = cut.Find(".chat-message-content-input");
        IElement sendButton = cut.Find(".post-chat-message-button");

        string userName = "Dog";
        string messageContent = "";

        usernameInput.Change(userName);
        contentInput.Change(messageContent);
        sendButton.Click();

        Assert.Empty(cut.FindAll(".chat-message"));
        Assert.NotEmpty(cut.FindAll(".validation-message"));
    }

    [Fact]
    public void CannotPostEmptyUsernames()
    {
        IRenderedFragment cut = RenderComponent<App>();
        IElement usernameInput = cut.Find(".username-input");
        IElement contentInput = cut.Find(".chat-message-content-input");
        IElement sendButton = cut.Find(".post-chat-message-button");

        string expectedUserName = "";
        string messageContent = "Any excuse will serve a tyrant.";

        usernameInput.Change(expectedUserName);
        contentInput.Change(messageContent);
        sendButton.Click();

        Assert.Empty(cut.FindAll(".chat-message"));
        Assert.NotEmpty(cut.FindAll(".validation-message"));
    }
}

W sumie ciekawy problem. W typowym backendowym kodzie by gdzieś była metoda PostMessage i to ją byśmy testowali. Ale że w tym wypadku mamy testy UI, to czegoś takiego nie ma, przez co faza act jest spora. Chyba najmniejszym złem jest dodać tę metodę pomocniczą.

Ale nie zrobiłem tego świadomie, dlatego, że gdzieś wyczytałem, że do testów odnoszą się zupełnie inne zasady, niż do "normalnego" kodu: testy winny być w największym możliwym stopniu pozbawione logiki i napisane najprościej, jak to możliwe, by można było łatwo zobaczyć powód, dlaczego dany test nie przechodzi patrząc tylko na jego kod (a nie jeszcze na kod iluśtam pomocniczych metod i klas) i by nie musieć pisać testów testujących testy.

Trochę dziwne, bo na dłuższą metę takie podejście wydaje się być nie do utrzymania w przyszłości. Skoro piszemy DSLe do fazy arrange, to czemu nie napisać metody pomoczniczej do act? Co prawda debugowanie będzie trudniejsze w przypadku nieprzechodzącego testu, ale idąc tym tokiem rozumowania, to nie powinniśmy też dzielić "zwykłego" kodu na metody, bo jak pojawi się błąd, to trzeba będzie wejść do środka metody zamiast od razu zobaczyć, gdzie jest błąd. "Wady" i zalety z wydzielania metod wydają się takie same zarówno w przypadku kodu testującego, jak i kodu testowanego.

Jest to całkiem normalne. Gdyby ufać tym labelkom z githuba, które pokazują ile % kodu w projekcie jest napisane w danym języku to by się okazało, że w mojej pracy piszemy wszystkie projekty w groovym. Projekty są oczywiście pisane w Javie + Spock w testach.

Oczywiście, gdy dużo kodu się powtarza w testach to warto to wydzielić do osobnego kodu pomocniczego, jakiegoś DSLa itd.

Rozmiar testów przebija rozmiar kodu?

Jeśli chcesz zachować rozsądne pokrycie to tak, to normalna sytuacja. Mój mały projekt https://github.com/pdy/simpleopenssl ma ~3620 wierszy plus testy ~7600 wierszy ;) Czyli ponad dwa razy więcej. W jednym korpo projekcie w którym uczestniczyłem ta różnica była jeszcze wyraźniejsza.

Tym nie mniej w Twoim przypadku widzę, że możesz uwspólnić pewne rzeczy. Nie znam biblioteki, której używasz, ale na pewno ma coś na zasadzie Test Fixture z google test, gdzie definiujesz sobie klasę testów która woła konkretne metody przed rozpoczęciem każdegu testu przez co możesz przygotować sobie wspólne dane dla testu. Wygląda to mniej więcej tak https://github.com/pdy/simpleopenssl/blob/master/test/x509/X509SignVerifyUT.cpp

Innym pojęciem jest value parametrized test gdzie masz jeden lub kilka testów i karmisz je różnymi danymi. Dzięki temu nie musisz kopiować w kółko tych samych test case'ów by sprawdzić działanie jakiegoś mechanizmu z innymi danymi wejściowymi. Wygląda to mniej wiecej tak https://github.com/pdy/simpleopenssl/blob/master/test/hash/HashBytesUT.cpp

Nie znam biblioteki, której używasz

bUnit / xUnit

ale na pewno ma coś na zasadzie Test Fixture z google test

Nie do końca widzę, co to da. Jeśli się nie mylę, fixtures są po to, by przygotować wspólne zależności, których nie można tworzyć na nowo przy każdej metodzie ze względu na wydajność. Nie jest wskazane (tak przynajmniej czytałem), by testy operowały na ten samej instancji testowanej klasy, bo to łamie izolację testów. A nawet, gdyby zignorować to zalecenie to też nie mogę wprowadzić jednej wiadomości do chatboxa i uruchamiać testy na tym chatboxie, bo jeden test chce mieć pustego chatboxa, a inny test wprowadza nieprawidłową wiadomość.

Innym pojęciem jest value parametrized test

Znowu nie widzę, co to da. To narzędzie, według mojego rozumienia, przydaje się wtedy, gdy testy są naprawdę niemal identyczne z dokładnością do danych wejściowych. Znowu tu tak nie jest. Jeden test sprawdza, czy prawidłowa wiadomość się pojawia, inny, czy nieprawidłowa wiadomość się nie pojawia. To są różne testy, a nie te same dla różnych danych wejściowych.

YetAnohterone napisał(a):

Z kolei plik z testami do powyższego: 158 linii!

W pełni zasłużony wykrzyknik zaskoczenia, tylko 4 razy więcej, to niezły wynik. :)

To prawda, jest spora duplikacja kodu w testach. Możnaby ją usunąć np. w ten sposób, by wprowadzić pomocnicze metody PostMessage czy AssertMessageEqual. Ale nie zrobiłem tego świadomie, dlatego, że gdzieś wyczytałem, że do testów odnoszą się zupełnie inne zasady, niż do "normalnego" kodu: testy winny być w największym możliwym stopniu pozbawione logiki i napisane najprościej, jak to możliwe, by można było łatwo zobaczyć powód, dlaczego dany test nie przechodzi patrząc tylko na jego kod (a nie jeszcze na kod iluśtam pomocniczych metod i klas) i by nie musieć pisać testów testujących testy.

Tyle, że aby kod dało się czytać, to trzeba go dzielić na funkcje, bo w przeciwnym razie mamy tasiemiec, w którym musimy schodzić na zbyt niskie poziomy abstrakcji, które rozpraszają i utrudniają zrozumienie tego, co kod robi.
Np. SecondMessageAppendsToTheFirstOne byłby bardziej zrozumiały, gdyby wyglądał tak:

[Fact]
public void SecondMessageAppendsToTheFirstOne()
{
    string firstExpectedUserName = "Wolf";
    string firstExpectedMessageContent = "How dare you muddle the water from which I am drinking.";
    string secondExpectedUserName = "Lambikin";
    string secondExpectedMessageContent = "Nay, master, nay, if the water be muddy up there, I cannot be the cause of it, for it runs down from you to me.";

    SendMessage(firstExpectedUserName, firstExpectedMessageContent);
    SendMessage(secondExpectedUserName, secondExpectedMessageContent);

    AssertFirstMessage(firstExpectedUserName, firstExpectedMessageContent);
    AssertSecondMessage(secondExpectedUserName, secondExpectedMessageContent);
}

Rozmiar testów przebija rozmiar kodu?

Nie ma w tym fakcie nic dziwnego. Powiedzmy, że będziesz miał funkcję jednolinijkową (np. liczącą silnię). Taką funkcję trzeba przetestować na ileś różnych sposobów, więc na jedną linijkę kodu produkcyjnego przypadnie np. z kilkanaście linijek, które testują wszystkie przypadki / edge case'y.

Czyli dajmy na to:

const factorial = x => x < 1? 1 : factorial(x - 1) * x

moglibyśmy ją przetestować tak (na pałę):

assert(factorial(0) == 1);
assert(factorial(1) == 1); 
assert(factorial(2) == 2); 
assert(factorial(3) == 6); 
assert(factorial(4) == 24); 
assert(factorial(5) == 120); 

albo tak (będąc bardziej sprytnym):

const expected = [1, 1, 2, 6, 24, 120];
expected.forEach((v, n) => {
    assert(factorial(n) === v); 
})

I już z jednej linijki kodu robi się ileś linijek testów. A nawet nie przetestowaliśmy tego do końca, tylko jedynie na liczbach całkowitych od 0 do 5. Plus boilerplate frameworka testowego trzeba uwzględnić, czego tutaj nie zrobiłem).

Albo np. masz funkcję, gdzie jest jakiś if i która może się różnie zachowywać w zależności od flagi. I znowu - to mogą być dwie linijki w kodzie:

if (condition) {
   doSomething();
}

ale zapewne żeby to przetestować, to zrobisz 2 przypadki testowe, gdzie w jednym powinno wpaść w ifa, a w drugim nie wpadnie. I to pewnie też będzie więcej niż 2 linijki kodu, które będziesz miał na produkcji.