Jak wiele testować w teście integracyjnym?

Cześć,
piszę właśnie testy integracyjne dla procesu rejestracji użytkownika i mam taki dylemat. Zastanawiam się jaki zakres funkcjonalności powinien obejmować pojedynczy test.

W przykładzie poniżej, test integracyjny obejmuje proces rejestracji, gdzie sprawdzam, czy użytkownik jest poprawnie zapisywany w bazie danych, czy podane username i email są zgodne z danymi przy rejestracji, oraz czy odpowiedni komunikat jest zwracany przez system. Czy według Was takie podejście jest odpowiednie? Czy może powinienem podzielić ten test na mniejsze?

@Test
    void shouldSuccessfullyRegisterAndSaveUser() throws DatabaseException {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "testUniqueEmail@gmail.com", "testPassword@123");

        ApiResponse response = registrationService.register(registerRequest);

        Optional<User> registeredUser = userDAO.findByUsername(registerRequest.username());

        assertTrue(registeredUser.isPresent());
        assertEquals(registerRequest.username(), registeredUser.get().getUsername());
        assertEquals(registerRequest.email(), registeredUser.get().getEmail());
        assertEquals("User registered successfully.", response.message());
    }

Z góry dzięki za pomoc.

Ornstein napisał(a):

piszę właśnie testy integracyjne dla procesu rejestracji użytkownika i mam taki dylemat. Zastanawiam się jaki zakres funkcjonalności powinien obejmować pojedynczy test.

Najmniejszy jaki tylko się da. Widać że ten test rzekomo testuje więcej niż jedną rzecz.

Ja bym to jakoś tak napisał:

@Test
void testRegisterUser() {
    registrationService.register(new RegisterRequest("kate", "kate@gmail.com", "pass"));
    assertUserExists("kate", "kate@gmail.com");
}

@Test
void testUniqueUsername() {
    registrationService.register(new RegisterRequest("duplicate", "mail1@gmail.com", "pass"));
    ApiResponse response = registrationService.register(new RegisterRequest("duplicate", "mail2@gmail.com", "pass"));
    assertUniqueUsername("duplicate"); 
}

@Test
void testMessage() {
    ApiResponse response = registrationService.register(new RegisterRequest("mark", "mark@mail.com", "pass"));
    assertEquals("User registered successfully.", response.message());
}

void assertUserExists(String username, String mail) {
    Optional<User> registeredUser = userDAO.findByUsername("kate");
    assertTrue(registeredUser.isPresent());
    assertEquals(username, registeredUser.get().getUsername());
    assertEquals(mail, registeredUser.get().getEmail());
}

void assertUniqueUsername(String username) {
  // jakoś tutaj się upewnij że user nie jest dodany dwa razy do bazy
}

Testy nadal nie są idealne, nadal można je poprawić - ale w kwestii podziału odpowiedzialności, tak można to podzielić.

Jeśli hasło ma wymagać pewnej struktury, to dopisz kolejny test, który sprawdza czy nie dało się zarejestrować usera, jak ktoś podaje hasło bez koniecznych znaków.

Mi się te testy w ogóle nie podobają.

Weryfikacja następuje nie przez zewnętrzną funkcję, a poprzez funkcję w samej aplikacji.

Potencjalny błąd w funkcji findByUsername da 'false negative' funkcji 'register'.

Zakładając, że funkcja 'register' zamienia 'gmail.com' na 'gmail.com.pl', i to samo robi funkcja 'findByUsername' (W druga strone) to wszystko wygląda fajnie, a dane są zepsute.

testRegisterUser jest ok, ale weryfikacja powinna być przez zewnętrzną testową funkcję bez żadnego ORM itd.

dmw napisał(a):

Zakładając, że funkcja 'register' zamienia 'gmail.com' na 'gmail.com.pl', i to samo robi funkcja 'findByUsername' (W druga strone) to wszystko wygląda fajnie, a dane są zepsute.
Potencjalny błąd w funkcji findByUsername da 'false negative' funkcji 'register'.

Jeśli chcesz to zamiast assertUserExists(username,mail), można zawołać assertUserExists(id,username,mail) która wyciąga usera po id. Szczegół. Można zmienić bez problemu.

dmw napisał(a):

testRegisterUser jest ok, ale weryfikacja powinna być przez zewnętrzną testową funkcję bez żadnego ORM itd.

Tak.

Riddle napisał(a):

Jeśli hasło ma wymagać pewnej struktury, to dopisz kolejny test, który sprawdza czy nie dało się zarejestrować usera, jak ktoś podaje hasło bez koniecznych znaków.

To zależy od stylu pisanych testów. Jak ktoś lubi testować takie combo to BDD jest fajne https://kotest.io/docs/framework/testing-styles.html#behavior-spec i takie testy fajnie pokazują, czy kolejne kroki się udały i jak rezultat jednego kroku wpływa na kolejne

slsy napisał(a):

Riddle napisał(a):

Jeśli hasło ma wymagać pewnej struktury, to dopisz kolejny test, który sprawdza czy nie dało się zarejestrować usera, jak ktoś podaje hasło bez koniecznych znaków.

To zależy od stylu pisanych testów. Jak ktoś lubi testować takie combo to BDD jest fajne https://kotest.io/docs/framework/testing-styles.html#behavior-spec i takie testy fajnie pokazują, czy kolejne kroki się udały i jak rezultat jednego kroku wpływa na kolejne

To co tutaj jest pokazane w tym linku to nie jest BDD, i obok BDD nie stało. To jest jedna rzecz. (Problemem tutaj jest przekonanie że BDD to są testy napisane w stylu "scenariuszowym".)

A druga rzecz to to, że takie testy są słabe bo testują jakiś szereg rzeczy, i dlatego to jest słabe, że jak jedna rzecz w tym szeregu nie działa, to cały "duży" test failuje. Nie widać wtedy jaka jest przyczyna tego błędu, i ciężko go naprawić. Mało tego, jeśli masz kilka testów napisanych w takim "scenariuszowym" stylu, to potem jeśli klient przyjdzie, i chce zmienić jakiś jeden krok, to wszystkie testy napisane w takim stylu wymagają zmiany.

Riddle napisał(a):

A druga rzecz to to, że takie testy są słabe bo testują jakiś szereg rzeczy, i dlatego to jest słabe, że jak jedna rzecz w tym szeregu nie działa, to cały "duży" test failuje. Nie widać wtedy jaka jest przyczyna tego błędu, i ciężko go naprawić. Mało tego, jeśli masz kilka testów napisanych w takim "scenariuszowym" stylu, to potem jeśli klient przyjdzie, i chce zmienić jakiś jeden krok, to wszystkie testy napisane w takim stylu wymagają zmiany.

To jak chcesz przetestować w zgrabny sposób taki scenariusz:

• wrzucam zasób poprzez POST/ i czytam go za pomocą GET /id
• usuwam zasób poprzez DELETE /id i spradzam, że GET /id zwraca 404

Jedyne co mi przychodzi do głowy to duplikacja tj. mam jeden test na POST + GET i drugi na POST + DELETE + GET, ale osobiście taki pełny scenariusz testujący wszystko wydaje mi się bardziej elegancki

slsy napisał(a):

To jak chcesz przetestować w zgrabny sposób taki scenariusz:

• wrzucam zasób poprzez POST/ i czytam go za pomocą GET /id
• usuwam zasób poprzez DELETE /id i spradzam, że GET /id zwraca 404

Problemem tutaj jest to, że ten POST robi dwie rzeczy na raz. W jednym przypadku testowym ten POST jest testowany, więc on powinien być najważniejszy. W drugim, ten POST jest tylko setupem stanu pod test, i powinien być nieistotny.

Dodawanie powinno mieć swój test, usuwanie powinno mieć swój osobny test.

To powinny być osobne testy, które wyglądają jakoś tak:

void testReadMissing() {
  assertResourceNotExists("jakaś dana");
}

void testCreate() {
  createResource("jakaś dana");
  assertResourceExists("jakaś dana");
}

void testDelete() {
  givenResourceExists("jakaś dana");
  deleteResource("jakaś dana");
  assertResourceNotExists("jakaś dana");
}

void testDeleteMissing() {
  response = deleteResource("jakaś dana");
  assertResponseFailure(response);
}

I owszem, możliwe że createResource() oraz givenResourceExists() mają pod spodem tego samego POSTa, możliwe że nawet z tymi samymi parametrami, ale to jest zamierzone.

Jakbym miał takie testy testReadMissing(), testCreate(), testDelete(), testDeleteMissing() to nigdy bym ich nie połączył w jakiegoś jednego frankensteina. Jak słyszałeś kiedyś powiedzenie "testy powinny być niezależne od siebie", to właśnie tego dotyczą. Jak napiszesz oba te testy w jednym, to właśnie dodałeś zależność testu tworzenia od testu usuwania.

slsy napisał(a):

Jedyne co mi przychodzi do głowy to duplikacja tj. mam jeden test na POST + GET i drugi na POST + DELETE + GET, ale osobiście taki pełny scenariusz testujący wszystko wydaje mi się bardziej elegancki

Może wydaje się, ale nie jest. Każda funkcjonalność powinna mieć osobny test. Nie powinieneś łączyć testów na tworzenie i usuwanie zasobu, tylko dlatego że oba używają tego samego POSTa. Poza tym, ten "pełny scenariusz testujący" sprawdzi Ci tylko happypatha.

dmw napisał(a):

Mi się te testy w ogóle nie podobają.

Weryfikacja następuje nie przez zewnętrzną funkcję, a poprzez funkcję w samej aplikacji.

Potencjalny błąd w funkcji findByUsername da 'false negative' funkcji 'register'.

Zakładając, że funkcja 'register' zamienia 'gmail.com' na 'gmail.com.pl', i to samo robi funkcja 'findByUsername' (W druga strone) to wszystko wygląda fajnie, a dane są zepsute.

testRegisterUser jest ok, ale weryfikacja powinna być przez zewnętrzną testową funkcję bez żadnego ORM itd.

Nie. To zależy.
Zależy jak sformułowane są wymagania.
Jeśli wymaganie jest takie, że "rejestracja powinna zapisać dane użytkownika w tabeli x w postaci y" to zgadzam się.
Ale to raczej rzadki i w sumie dość patologiczny przypadek (czemu niby kogoś ma obchodzić, gdzie i jak to jest zapisane).

Natomiast jeśli wymaganie jest, że "zarejestrowany użytkownik powinien móc się zalogować" to używamy rejestracji i testujemy funkcją do logowania.

To, że w razie błędu więcej testów się wywali nie jest żadnym problemem, normalka - nic złego się nie dzieje. Mi się przed chwilą wywaliło jakieś 200 testów w projekcie -> pomyliłem się w domyślnym konfigu. To, że walnęło tych testów 200 od razu powiedziało mi gdzie szukać problemu :-)

Tak lepiej ? Co myślicie ?

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ActiveProfiles("test")
public class RegistrationServiceIntegrationTest {
    @Autowired
    private RegistrationService registrationService;
    @Autowired
    private DataSource dataSource;
    @Autowired
    private LoginService loginService;
    @BeforeEach
    void init() throws Exception {
        try(Connection connection = dataSource.getConnection()) {
            ScriptUtils.executeSqlScript(connection, new ClassPathResource("data_sql/test-data-setup.sql"));
        }
    }
    @AfterEach
    void cleanup() {
        JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource);
        jdbcTemplate.update("DELETE FROM user_roles");
        jdbcTemplate.update("DELETE FROM users");
        jdbcTemplate.update("DELETE FROM roles");
    }
    @Test
    void shouldRegisterUserAndAllowLoginWithValidCredentials() throws DatabaseException {
        String username = "testUniqueUsername";
        String email = "testUniqueEmail@gmail.com";
        String password = "testPassword@123";
        RegisterRequest registerRequest = new RegisterRequest(username, email, password);

        registrationService.register(registerRequest);

        LoginRequest loginRequest = new LoginRequest(username, password);

        String token = loginService.login(loginRequest);
        assertNotNull(token, "JWT token has not been generated");
    }

    @Test
    void shouldAssignRoleUserToNewUser() throws DatabaseException, SQLException {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "testUniqueEmail@gmail.com", "testPassword@123");

        registrationService.register(registerRequest);

        assertTrue(assertUserRoleAddedToNewUser(registerRequest.username(), UserRole.ROLE_USER.name()));
    }
    @Test
    void shouldReturnSuccessMessageOnUserRegistration () throws DatabaseException {
        String username = "testUniqueUsername";
        String email = "testUniqueEmail@gmail.com";
        String password = "testPassword@123";
        RegisterRequest registerRequest = new RegisterRequest(username, email, password);

        ApiResponse response = registrationService.register(registerRequest);

        assertEquals("User registered successfully.", response.message());
    }
    @Test
    void shouldThrowExceptionForInvalidUsernameFormatOnRegistration() {
        RegisterRequest registerRequest = new RegisterRequest(
                "invalidUsername$Format", "testUniqueEmail@gmail.com", "testPassword@123");

        assertThrows(CredentialValidationException.class, () -> registrationService.register(registerRequest));
    }
    @Test
    void shouldReturnErrorMessageForInvalidUsernameFormatOnRegistration() {
        RegisterRequest registerRequest = new RegisterRequest(
                "invalidUsername$Format", "testUniqueEmail@gmail.com", "testPassword@123");

        Exception exception = assertThrows(CredentialValidationException.class,
                () -> registrationService.register(registerRequest));

        String expectedMessage = "Invalid username format. The username can contain only letters and numbers," +
                " and should be between 3 and 20 characters long.";

        assertEquals(expectedMessage, exception.getMessage());
    }
    @Test
    void shouldThrowExceptionForInvalidEmailFormatOnRegistration() {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "invalidEmailFormat", "testPassword@123");

        assertThrows(CredentialValidationException.class, () -> registrationService.register(registerRequest));
    }
    @Test
    void shouldReturnErrorMessageForInvalidEmailFormatOnRegistration() {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "invalidEmailFormat", "testPassword@123");

        Exception exception = assertThrows(CredentialValidationException.class,
                () -> registrationService.register(registerRequest));

        String expectedMessage = "Invalid email address format. The email should follow the standard format" +
                " (e.g., user@example.com) and be between 6 and 30 characters long.";

        assertEquals(expectedMessage, exception.getMessage());
    }
    @Test
    void shouldThrowExceptionForInvalidPasswordFormatOnRegistration() {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "testUniqueEmail@gmail.com", "invalidFormat");

        assertThrows(CredentialValidationException.class, () -> registrationService.register(registerRequest));
    }
    @Test
    void shouldReturnErrorMessageForInvalidPasswordFormat() {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "testUniqueEmail@gmail.com", "invalidFormat");

        Exception exception = assertThrows(CredentialValidationException.class,
                () -> registrationService.register(registerRequest));

        String expectedMessage = "Invalid password format. The password must contain an least 8 characters, " +
                "including uppercase letters, lowercase letters, numbers, and special characters.";

        assertEquals(expectedMessage, exception.getMessage());
    }
    @Test
    void shouldThrowExceptionWhenTryRegisterWithExistingUsername() {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUsername", "testUniqueEmail@gmail.com", "testPassword@123");

        assertThrows(CredentialValidationException.class, () -> registrationService.register(registerRequest));
    }
    @Test
    void shouldReturnErrorMessageWhenTryRegisterWithExistingUsername() {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUsername", "testUniqueEmail@gmail.com", "testPassword@123");

        Exception exception =assertThrows(CredentialValidationException.class,
                () -> registrationService.register(registerRequest));

        String expectedMessage = "Username is already exist!.";

        assertEquals(expectedMessage, exception.getMessage());
    }
    @Test
    void shouldThrowExceptionWhenTryRegisterWithExistingEmail() {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "testEmail@gmail.com", "testPassword@123");

        assertThrows(CredentialValidationException.class, () -> registrationService.register(registerRequest));
    }
    @Test
    void shouldReturnErrorMessageWhenTryRegisterWithExistEmail() {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "testEmail@gmail.com", "testPassword@123");

        Exception exception = assertThrows(CredentialValidationException.class,
                () -> registrationService.register(registerRequest));

        String expectedMessage = "Email is already exist!.";

        assertEquals(expectedMessage, exception.getMessage());
    }
    private boolean assertUserRoleAddedToNewUser(String username, String roleName) throws SQLException {
        String query = "SELECT COUNT(*) " +
                "FROM user_roles ur " +
                "JOIN users u ON ur.user_id = u.id " +
                "JOIN roles r ON ur.role_id = r.id " +
                "WHERE u.username = ? AND r.name = ?";

        try (Connection connection = dataSource.getConnection();
             PreparedStatement statement = connection.prepareStatement(query)) {

            statement.setString(1, username);
            statement.setString(2, roleName);

            ResultSet resultSet = statement.executeQuery();

            return resultSet.next() && resultSet.getInt(1) == 1;
        }
    }
}

jarekr000000 napisał(a):

dmw napisał(a):

Mi się te testy w ogóle nie podobają.

Weryfikacja następuje nie przez zewnętrzną funkcję, a poprzez funkcję w samej aplikacji.

Potencjalny błąd w funkcji findByUsername da 'false negative' funkcji 'register'.

Zakładając, że funkcja 'register' zamienia 'gmail.com' na 'gmail.com.pl', i to samo robi funkcja 'findByUsername' (W druga strone) to wszystko wygląda fajnie, a dane są zepsute.

testRegisterUser jest ok, ale weryfikacja powinna być przez zewnętrzną testową funkcję bez żadnego ORM itd.

Nie. To zależy.
Zależy jak sformułowane są wymagania.
Jeśli wymaganie jest takie, że "rejestracja powinna zapisać dane użytkownika w tabeli x w postaci y" to zgadzam się.
Ale to raczej rzadki i w sumie dość patologiczny przypadek (czemu niby kogoś ma obchodzić, gdzie i jak to jest zapisane).

@jarekr000000 wg. mnie to nie jest rzadki i patologiczny przypadek. Jeżeli nie testujemy jak zapisane są dane, to ktoś może sobie wrzucić AtttributeConvertera lub jakieś inne ustrojstwo i dane zwracane przez aplikację wyglądają poprawnie ale różnią się od tego co jest w bazie. Od tego momentu masz ściśle powiązanie pomiędzy cyklem życia danych a cyklem życia aplikacji.

jarekr000000 napisał(a):

Natomiast jeśli wymaganie jest, że "zarejestrowany użytkownik powinien móc się zalogować" to używamy rejestracji i testujemy funkcją do logowania.

OK, czyli rejestruję się z mailem dmw.@wp.pl siedzi ten attributeConverter i zmienia mi go na dmw_@wp_pl i tak zapisuję w bazie, potem jak się loguję to znowu wchodzi attributeConverter i zmienia _ na "." z bazy, test jest zielony a w bazie danych jest śmietnik.

Inna sprawa, że dzisiaj dane nie żyją tylko w konkretnej aplikacji, ale też w hurtowniach, propagowane są przez eventy itd. także dla mnie test powinien sprawdzać co jest w bazie, jak rowniez co zwraca aplikacja.

Argument nietechniczny, jednak ważny. Wydaje mi się, że jeżeli zapiszesz dane w innej formie aniżeli podał je użytkownik to wchodzisz na wąski lód, jeżeli chodzi o RODO.

Ornstein napisał(a):

Kopiuj

    @Test
    void shouldRegisterUserAndAllowLoginWithValidCredentials() throws DatabaseException {
        String username = "testUniqueUsername";
        String email = "testUniqueEmail@gmail.com";
        String password = "testPassword@123";
        RegisterRequest registerRequest = new RegisterRequest(username, email, password);

        registrationService.register(registerRequest);

        LoginRequest loginRequest = new LoginRequest(username, password);

        String token = loginService.login(loginRequest);
        assertNotNull(token, "JWT token has not been generated");
    }

Nie wiem co ten test testuje? Wygląda jakby testował rejestrowanie i logowanie? Jeśli tak, to powinien być rozbity na dwa testy. np tak:

@Test
void shouldRegisterUser() throws DatabaseException {
    registrationService.register(JakiśHelper.newUserRegisterRequest());
    // tutaj asercja żę konto istnieje
}

@Test
void shouldRegisterUserAndAllowLoginWithValidCredentials() throws DatabaseException {
    jakiśHelper.givenAnExistingUser();
    assertNotNull(loginService.login(jakiśHelper.userLoginRequest()));
}

Kopiuj

    @Test
    void shouldAssignRoleUserToNewUser() throws DatabaseException, SQLException {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "testUniqueEmail@gmail.com", "testPassword@123");

        registrationService.register(registerRequest);

        assertTrue(assertUserRoleAddedToNewUser(registerRequest.username(), UserRole.ROLE_USER.name()));
    }

Znowu niepotrzebne szczegóły w teście:

@Test
void shouldAssignRoleUserToNewUser() throws DatabaseException, SQLException {
  registrationService.register(JakiśHelper.newUserRegisterRequest("mark"));
  assertEquals(userRole("mark"), UserRole.ROLE_USER));
}

W pozostałych testach, też dobrze jakbyś użył helpera. Za dużo szczegółów jest w każdym z tych testów, przez co ciężko się je czyta.

Testy:

• shouldReturnErrorMessageForInvalidPasswordFormat
• shouldReturnErrorMessageForInvalidEmailFormatOnRegistration
• shouldReturnErrorMessageWhenTryRegisterWithExistEmail (czemu pozostałe testy mają w nazwie For a ten ma When?)

Mają duplikacje. Wynieś części wspólne do osobnej metody.

dmw napisał(a):

Ale to raczej rzadki i w sumie dość patologiczny przypadek (czemu niby kogoś ma obchodzić, gdzie i jak to jest zapisane).

@jarekr000000 wg. mnie to nie jest rzadki i patologiczny przypadek. Jeżeli nie testujemy jak zapisane są dane, to ktoś może sobie wrzucić AtttributeConvertera lub jakieś inne ustrojstwo i dane zwracane przez aplikację wyglądają poprawnie ale różnią się od tego co jest w bazie. Od tego momentu masz ściśle powiązanie pomiędzy cyklem życia danych a cyklem życia aplikacji.

I co się wtedy dzieje? Bo nieważne jak zapisane, to skoro są odczytywalne to w czym jest problem?

jarekr000000 napisał(a):

Natomiast jeśli wymaganie jest, że "zarejestrowany użytkownik powinien móc się zalogować" to używamy rejestracji i testujemy funkcją do logowania.

OK, czyli rejestruję się z mailem dmw.@wp.pl siedzi ten attributeConverter i zmienia mi go na dmw_@wp_pl i tak zapisuję w bazie, potem jak się loguję to znowu wchodzi attributeConverter i zmienia _ na "." z bazy, test jest zielony a w bazie danych jest śmietnik.

Wiesz, że bazy danych zapisują to potem jako pliki (np. binarne) i na dysku masz śmietnik, bo zamiast dmw.@wp.pl siedzi tam sobie 0x2334349545fe32432..... I co teraz?

Inna sprawa, że dzisiaj dane nie żyją tylko w konkretnej aplikacji, ale też w hurtowniach, propagowane są przez eventy itd. także dla mnie test powinien sprawdzać co jest w bazie, jak rowniez co zwraca aplikacja.

Zdradzę Ci tajemnice -> czasem nawet aplikacja nie ma bazy, czasem w ogóle nie jest to baza SQL.

Argument nietechniczny, jednak ważny. Wydaje mi się, że jeżeli zapiszesz dane w innej formie aniżeli podał je użytkownik to wchodzisz na wąski lód, jeżeli chodzi o RODO.

Niby dlaczego, wiesz jak Twoje dane zapisuje jakakolwiek aplikacja? Facebook? mObywatel? Jako programiast nawet jak zapisujesz do bazy danych, to nie masz przeważnie kontroli jak to baza danych fizycznie to zapisuje.
Co więcej - jeśli chodzi o RODO itd, często dane identyfikujące, wrażliwe zapisuje się w postaci zaszyfrowanej, przy pomocy specjalnego dodatkowego serwisu.

Co do poprzedniego -> fajnie jak system jest zrobiony w ten sposob, ze bez wiekszych bolesci mozna przepiac sie z jednej bazy na inna. Wiec na poziomie testu integracyjnego nie powinno nas interesowac co sie zapisuje. Rozwiazanie bazodanowe powinno miec wlasne testy, inaczej rownie dobrze moglbys testowac czy JVMka albo procesor robi to czego oczekujesz. IMHO mozna, ale niekoniecznie ma to sens przy tescie integracyjnym. Bo pozniej konczy sie to suita kilku tyseicy testow ktore wlasciwie nie wiadomo co sprawdzaja, ich wykonanie trwa dwa dni i wszyscy maja wywalone czy failuje 70 czy 80%.

Ale wy wiecie że testami integracyjnymi się nie testuje logiki, nie?

jarekr000000 napisał(a):

dmw napisał(a):

Ale to raczej rzadki i w sumie dość patologiczny przypadek (czemu niby kogoś ma obchodzić, gdzie i jak to jest zapisane).

@jarekr000000 wg. mnie to nie jest rzadki i patologiczny przypadek. Jeżeli nie testujemy jak zapisane są dane, to ktoś może sobie wrzucić AtttributeConvertera lub jakieś inne ustrojstwo i dane zwracane przez aplikację wyglądają poprawnie ale różnią się od tego co jest w bazie. Od tego momentu masz ściśle powiązanie pomiędzy cyklem życia danych a cyklem życia aplikacji.

I co się wtedy dzieje? Bo nieważne jak zapisane, to skoro są odczytywalne to w czym jest problem?

Wszystko jest OK. Zmieniasz kod i musisz uaktualnić bazę danych ponieważ Twoje dane są ściśle powiązane z Twoim kodem. Z pewnością update produkcyjnej bazy danych to coś co jest bezbolesne także nie trzeba o tym myśleć :)

jarekr000000 napisał(a):

Natomiast jeśli wymaganie jest, że "zarejestrowany użytkownik powinien móc się zalogować" to używamy rejestracji i testujemy funkcją do logowania.

OK, czyli rejestruję się z mailem dmw.@wp.pl siedzi ten attributeConverter i zmienia mi go na dmw_@wp_pl i tak zapisuję w bazie, potem jak się loguję to znowu wchodzi attributeConverter i zmienia _ na "." z bazy, test jest zielony a w bazie danych jest śmietnik.
Wiesz, że bazy danych zapisują to potem jako pliki (np. binarne) i na dysku masz śmietnik, bo zamiast dmw.@wp.pl siedzi tam sobie 0x2334349545fe32432..... I co teraz?

Noi tutaj weszliśmy już na poziom dyskusji 'muszę mieć rację bo muszę mieć rację'. Nie rozmawiamy tutaj o tym jak bazy danych przechowują dane z technicznego punktu widzenia.

Inna sprawa, że dzisiaj dane nie żyją tylko w konkretnej aplikacji, ale też w hurtowniach, propagowane są przez eventy itd. także dla mnie test powinien sprawdzać co jest w bazie, jak rowniez co zwraca aplikacja.

Zdradzę Ci tajemnice -> czasem nawet aplikacja nie ma bazy, czasem w ogóle nie jest to baza SQL.

I co to ma do rzeczy? Czasem aplikacja nie ma bazy danych, czasem ma embedded, czasem ma SQL a czasem ma NoSQL. Jeżeli Twój kod modyfikuje dane i sprawia, że są one poprawne tylko w kontekście tej wersji Twojego kodu i tej aplikacji to jest to patologia.

Załóżmy, że masz jakaś implementację transactional outbound pattern. Zapisujesz dane w bazie z jakąkolwiek zmianą przez attribute convertera lub cokolwiek innego. Dane wyświetlane przez aplikację wyglądają poprawnie, ale przez eventy idzie coś zupełnie innego.

dmw napisał(a):

Wszystko jest OK. Zmieniasz kod i musisz uaktualnić bazę danych ponieważ Twoje dane są ściśle powiązane z Twoim kodem. Z pewnością update produkcyjnej bazy danych to coś co jest bezbolesne także nie trzeba o tym myśleć :)

Update bazy danych produkcyjnej to nie musi być jeden plik update.sql, nie musi to być nawet SQL.

Noi tutaj weszliśmy już na poziom dyskusji 'muszę mieć rację bo muszę mieć rację'. Nie rozmawiamy tutaj o tym jak bazy danych przechowują dane z technicznego punktu widzenia.

Robią dokładnie to samo, co uważasz za niepoprawne - składują dane w specyficzny dla aplikacji sposób. To samo tylko na innym poziomie.

I co to ma do rzeczy? Czasem aplikacja nie ma bazy danych, czasem ma embedded, czasem ma SQL a czasem ma NoSQL. Jeżeli Twój kod modyfikuje dane i sprawia, że są one poprawne tylko w kontekście tej wersji Twojego kodu i tej aplikacji to jest to patologia.

Załóżmy, że masz jakaś implementację transactional outbound pattern. Zapisujesz dane w bazie z jakąkolwiek zmianą przez attribute convertera lub cokolwiek innego. Dane wyświetlane przez aplikację wyglądają poprawnie, ale przez eventy idzie coś zupełnie innego.

I co z tego? Jeśli dane są własnością aplikacji to sposób ich składowania jest wewnętrzną sprawą aplikacji. Jest to zupełnie normalne. Tak samo jak to, że bazy potem trzymają te dane w sposób binarny, zupełnie nieczytelny z zewnątrz.

Ornstein napisał(a):

piszę właśnie testy integracyjne dla procesu rejestracji użytkownika i mam taki dylemat. Zastanawiam się jaki zakres funkcjonalności powinien obejmować pojedynczy test.

Nakłoń menedżment do wypuszczenia wersji produktu na jakimś nieznaczącym rynku (np. Republika Tuwy), niech QA zbierze feedback, spisze na jego podstawie przypadki i scenariusze i wtedy wróć ponownie z pytaniem.

jarekr000000 napisał(a):

dmw napisał(a):

Wszystko jest OK. Zmieniasz kod i musisz uaktualnić bazę danych ponieważ Twoje dane są ściśle powiązane z Twoim kodem. Z pewnością update produkcyjnej bazy danych to coś co jest bezbolesne także nie trzeba o tym myśleć :)

Update bazy danych produkcyjnej to nie musi być jeden plik update.sql, nie musi to być nawet SQL.

To jak wygląda update produkcyjnej bazy danych nie jest przedmiotem tej dyskusji. Sytuacja w której nie testujemy jak dane są zapisane w bazie(Bez żadnych ORM itd.), może skutkować tym, że nieświadomie rozjedzą się stan bazy danych z tym co zwraca aplikacja. To tworzy powiązanie między kodem a danymi z którego ciężko jest wyjść.

Noi tutaj weszliśmy już na poziom dyskusji 'muszę mieć rację bo muszę mieć rację'. Nie rozmawiamy tutaj o tym jak bazy danych przechowują dane z technicznego punktu widzenia.

Robią dokładnie to samo, co uważasz za niepoprawne - składują dane w specyficzny dla aplikacji sposób. To samo tylko na innym poziomie.

Ja mówię o zmianie znaczenia danych np. z "dmw.@wp.pl" na "dmw_@wp_pl". To co robią bazy danych to zmiana formatu danych a to zupełnie inna kwestia. W przypadku o którym rozmawiamy baza danych cały czas przechowywała by "dmw.@wp.pl" tylko w formie pliku binarnego. Nawet jezeli odczytasz bezposrednio plik binarny to caly czas bedzie dmw.@wp.pl tylko zapisane w innym formacie.

jarekr000000 napisał(a):

dmw napisał(a):

Załóżmy, że masz jakaś implementację transactional outbound pattern. Zapisujesz dane w bazie z jakąkolwiek zmianą przez attribute convertera lub cokolwiek innego. Dane wyświetlane przez aplikację wyglądają poprawnie, ale przez eventy idzie coś zupełnie innego.

I co z tego? Jeśli dane są własnością aplikacji to sposób ich składowania jest wewnętrzną sprawą aplikacji. Jest to zupełnie normalne. Tak samo jak to, że bazy potem trzymają te dane w sposób binarny, zupełnie nieczytelny z zewnątrz.

To czy dane są własnością aplikacji i co dokładnie to znaczy jest dość szerokim tematem. Niemniej załóżmy, że dane są własnością aplikacji. Co to zmienia w kwestii przykładu z transactional outbound? API zwraca dmw.@wp.pl a do reszty systemu idzie dmw_@wp_pl?

dmw napisał(a):

To jak wygląda update produkcyjnej bazy danych nie jest przedmiotem tej dyskusji. Sytuacja w której nie testujemy jak dane są zapisane w bazie(Bez żadnych ORM itd.), może skutkować tym, że nieświadomie rozjedzą się stan bazy danych z tym co zwraca aplikacja. To tworzy powiązanie między kodem a danymi z którego ciężko jest wyjść.

To jest normalne powiązanie - jeśli baza należy do aplikacji (a to norma) to aplikacja decyduje jak składuje dane -> inaczej to jest patologia.

Ja mówię o zmianie znaczenia danych np. z "dmw.@wp.pl" na "dmw_@wp_pl". To co robią bazy danych to zmiana formatu danych a to zupełnie inna kwestia. W przypadku o którym rozmawiamy baza danych cały czas przechowywała by "dmw.@wp.pl" tylko w formie pliku binarnego. Nawet jezeli odczytasz bezposrednio plik binarny to caly czas bedzie dmw.@wp.pl tylko zapisane w innym formacie.

Nie, nie zgadzam się. To jest dokładnie ten sam przypadek tylko na innym poziomie aplikacji. Ty masz w aplikacji stringa a w bazie jest jakiś VARCHAR to już od razu konwersja i może ona być całkiem nitrywialna, jeśli trzeba. Czasem przecież jest to nawet jakiś BLOB.

jarekr000000 napisał(a):

To czy dane są własnością aplikacji i co dokładnie to znaczy jest dość szerokim tematem. Niemniej załóżmy, że dane są własnością aplikacji. Co to zmienia w kwestii przykładu z transactional outbound? API zwraca dmw.@wp.pl a do reszty systemu idzie dmw_@wp_pl?

A niby jakim cudem do reszty systemu miałoby iść dmw_@wp_pl skoro to aplikacja odczytuje? I w takim razie robi odpowiednią konwersję.
Jeśli nie zrobi tej konwersji to wywali się na teście (od tego się zaczęło) i będzie widać, że jest problem.

Btw. żebyśmy nie szli w paranoje - nie polecam, ani nie promuje jakiegoś sztucznowego manglowania danych. Cały czas chodzi mi tylko o to, że testowanie
tego co jest w bazie danych, zamiast używania API aplikacji w teście to raczej słaby pomysł, niepotrzebna komplikacja (jak pisałem zależy od tego jak są zdefiniowane wymagania i jakie jest to API).

Riddle napisał(a):

Ornstein napisał(a):

Kopiuj

    @Test
    void shouldRegisterUserAndAllowLoginWithValidCredentials() throws DatabaseException {
        String username = "testUniqueUsername";
        String email = "testUniqueEmail@gmail.com";
        String password = "testPassword@123";
        RegisterRequest registerRequest = new RegisterRequest(username, email, password);

        registrationService.register(registerRequest);

        LoginRequest loginRequest = new LoginRequest(username, password);

        String token = loginService.login(loginRequest);
        assertNotNull(token, "JWT token has not been generated");
    }

Nie wiem co ten test testuje? Wygląda jakby testował rejestrowanie i logowanie? Jeśli tak, to powinien być rozbity na dwa testy. np tak:

Kopiuj

@Test
void shouldRegisterUser() throws DatabaseException {
    registrationService.register(JakiśHelper.newUserRegisterRequest());
    // tutaj asercja żę konto istnieje
}

Kopiuj

@Test
void shouldRegisterUserAndAllowLoginWithValidCredentials() throws DatabaseException {
    jakiśHelper.givenAnExistingUser();
    assertNotNull(loginService.login(jakiśHelper.userLoginRequest()));
}

Kopiuj

    @Test
    void shouldAssignRoleUserToNewUser() throws DatabaseException, SQLException {
        RegisterRequest registerRequest = new RegisterRequest(
                "testUniqueUsername", "testUniqueEmail@gmail.com", "testPassword@123");

        registrationService.register(registerRequest);

        assertTrue(assertUserRoleAddedToNewUser(registerRequest.username(), UserRole.ROLE_USER.name()));
    }

Znowu niepotrzebne szczegóły w teście:

Kopiuj

@Test
void shouldAssignRoleUserToNewUser() throws DatabaseException, SQLException {
  registrationService.register(JakiśHelper.newUserRegisterRequest("mark"));
  assertEquals(userRole("mark"), UserRole.ROLE_USER));
}

W pozostałych testach, też dobrze jakbyś użył helpera. Za dużo szczegółów jest w każdym z tych testów, przez co ciężko się je czyta.

Testy:

• shouldReturnErrorMessageForInvalidPasswordFormat
• shouldReturnErrorMessageForInvalidEmailFormatOnRegistration
• shouldReturnErrorMessageWhenTryRegisterWithExistEmail (czemu pozostałe testy mają w nazwie For a ten ma When?)

Mają duplikacje. Wynieś części wspólne do osobnej metody.

Teraz lepiej ?

Klasa testowa:

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@ActiveProfiles("test")
public class RegistrationServiceIntegrationTest {
    @Autowired
    private RegistrationService registrationService;
    @Autowired
    private DataSource dataSource;
    @Autowired
    private DatabaseHelper databaseHelper;
    @Autowired
    private LoginService loginService;
    @BeforeEach
    void init() throws Exception {
        try(Connection connection = dataSource.getConnection()) {
            ScriptUtils.executeSqlScript(connection, new ClassPathResource("data_sql/test-data-setup.sql"));
        }
    }
    @AfterEach
    void cleanup() {
        JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource);
        jdbcTemplate.update("DELETE FROM user_roles");
        jdbcTemplate.update("DELETE FROM users");
        jdbcTemplate.update("DELETE FROM roles");
    }
    @Test
    void shouldRegisterUser() throws DatabaseException, SQLException {
        RegisterRequest request = RegistrationHelper.createValidRegisterRequestWithUsername("testUsername");
        registrationService.register(request);
        assertTrue(databaseHelper.doesUserExist(request.username()));
    }
    @Test
    void shouldAllowLoginForRegisterUser() throws DatabaseException {
        RegistrationHelper.executeValidUserRegistration(registrationService);
        assertNotNull(loginService.login(RegistrationHelper.createUserLoginRequest()));
    }

    @Test
    void shouldAssignRoleUserToNewUser() throws DatabaseException, SQLException {
        registrationService.register(RegistrationHelper.createValidRegisterRequestWithUsername("testUsername"));
        assertTrue(databaseHelper.hasUserRole("testUsername", UserRole.ROLE_USER.name()));
    }
    @Test
    void shouldReturnSuccessMessageOnUserRegistration () throws DatabaseException {
        ApiResponse response = registrationService.register(RegistrationHelper.createValidRegisterRequest());
        assertEquals(TestMessages.SUCCESS_REGISTRATION, response.message());
    }
    @Test
    void shouldThrowExceptionForInvalidUsernameFormat() {
        assertValidationException(() -> registrationService.register(
                RegistrationHelper.createInvalidUsernameRequest()));
    }
    @Test
    void shouldReturnErrorMessageForInvalidUsernameFormat() {
       assertErrorMessage(() -> registrationService.register(
               RegistrationHelper.createInvalidUsernameRequest()), TestMessages.INVALID_USERNAME_FORMAT);
    }
    @Test
    void shouldThrowExceptionForInvalidEmailFormat() {
        assertValidationException(() -> registrationService.register(
                RegistrationHelper.createInvalidEmailRequest()));
    }
    @Test
    void shouldReturnErrorMessageForInvalidEmailFormat() {
        assertErrorMessage(() -> registrationService.register(
                RegistrationHelper.createInvalidEmailRequest()), TestMessages.INVALID_EMAIL_FORMAT);
    }
    @Test
    void shouldThrowExceptionForInvalidPasswordFormat() {
        assertValidationException(() -> registrationService.register(
                RegistrationHelper.createInvalidPasswordRequest()));
    }
    @Test
    void shouldReturnErrorMessageForInvalidPasswordFormat() {
        assertErrorMessage(() -> registrationService.register(
                RegistrationHelper.createInvalidPasswordRequest()), TestMessages.INVALID_PASSWORD_FORMAT);
    }
    @Test
    void shouldThrowExceptionForExistUsername() {
        assertValidationException(() -> registrationService.register(
                RegistrationHelper.createExistUsernameRequest()));
    }
    @Test
    void shouldReturnErrorMessageForExistUsername() {
        assertErrorMessage(() -> registrationService.register(
                RegistrationHelper.createExistUsernameRequest()), TestMessages.USERNAME_EXIST);
    }
    @Test
    void shouldThrowExceptionForExistEmail() {
        assertValidationException(() -> registrationService.register(
                RegistrationHelper.createExistEmailRequest()));
    }
    @Test
    void shouldReturnErrorMessageForExistEmail() {
        assertErrorMessage(() -> registrationService.register(
                RegistrationHelper.createExistEmailRequest()), TestMessages.EMAIL_EXIST);
    }
}

Klasy pomocnicze:

public class UserAssertions {
    public static void assertValidationException(Executable executable) {
        assertThrows(CredentialValidationException.class, executable);
    }
    public static void assertErrorMessage(Executable executable, String expectedMessage) {
        Exception exception = assertThrows(Exception.class, executable);
        assertEquals(expectedMessage, exception.getMessage());
    }
}

public class RegistrationHelper {
    public static RegisterRequest createValidRegisterRequest() {
        return new RegisterRequest("testUsername", "testEmail@gmail.com", "testPassword@123");
    }
    public static LoginRequest createUserLoginRequest() {
        return new LoginRequest("testUsername", "testPassword@123");
    }
    public static RegisterRequest createValidRegisterRequestWithUsername(String username) {
        return new RegisterRequest(username, username + "@gmail.com", "testPassword@123");
    }
    public static RegisterRequest createInvalidUsernameRequest() {
        return new RegisterRequest(
                "invalidUsername$Format", "testEmail@gmail.com", "testPassword@123");
    }
    public static RegisterRequest createInvalidEmailRequest() {
        return new RegisterRequest(
                "testUsername", "invalidEmailFormat", "testPassword@123");
    }
    public static RegisterRequest createInvalidPasswordRequest() {
        return new RegisterRequest(
                "testUsername", "testEmail@gmail.com", "invalidPassword");
    }
    public static RegisterRequest createExistUsernameRequest() {
        return new RegisterRequest(
                "testExistUsername", "testEmail@gmail.com", "testPassword@123");
    }
    public static RegisterRequest createExistEmailRequest() {
        return new RegisterRequest(
                "testUsername", "testExistEmail@gmail.com", "testPassword@123");
    }
    public static void executeValidUserRegistration(RegistrationService registrationService) throws DatabaseException {
        registrationService.register(createValidRegisterRequest());
    }
}

@Component
public class DatabaseHelper {
    private final DataSource dataSource;
    public DatabaseHelper(DataSource dataSource) {
        this.dataSource = dataSource;
    }

    public boolean doesUserExist(String username) throws SQLException {
        String query = "SELECT COUNT(*) " +
                "FROM users " +
                "WHERE username = ?";

        try (Connection connection = dataSource.getConnection();
             PreparedStatement statement = connection.prepareStatement(query)) {

            statement.setString(1, username);

            ResultSet resultSet = statement.executeQuery();

            return resultSet.next() && resultSet.getInt(1) > 0;
        }
    }
     public boolean hasUserRole(String username, String roleName) throws SQLException {
        String query = "SELECT COUNT(*) FROM user_roles ur " +
                "JOIN users u ON ur.user_id = u.id " +
                "JOIN roles r ON ur.role_id = r.id " +
                "WHERE u.username = ? AND r.name = ?";

        try (Connection connection = dataSource.getConnection();
             PreparedStatement statement = connection.prepareStatement(query)) {

            statement.setString(1, username);
            statement.setString(2, roleName);
            ResultSet resultSet = statement.executeQuery();

            if (resultSet.next()) {
                return resultSet.getInt(1) > 0;
            } else {
                return false;
            }
        }
    }
}

Lepiej.

1. Ale testy powinny mieć jawne dane, więc test shouldRegisterUser() powinien wyglądać tak:

   Kopiuj

   @Test
   void shouldRegisterUser() {
       registrationService.register(RegistrationHelper.registerRequest("mark"));
       assertTrue(databaseHelper.doesUserExist("mark"));
   }
   

   I czemu doesUserExist()? Czemu nie po prostu userExists()?

2. Wywal z testów te throws DatabaseException, SQLException. Złap te wyjątki w helperach testowych i zamiast tego rzuć RuntimeException. Albo ewentualnie przerób DatabaseException na RuntimeException.

3. Init i teardown niepotrzebnie mają szczegóły bazy, przerób na:

   Kopiuj

   @BeforeEach
   void init() throws Exception {
       dataSource.setup("data_sql/test-data-setup.sql");
   }
   
   @AfterEach
   void cleanup() {
       dataSource.removeUsersAndRoles();
   }
   

4. Niepotrzebnie trzymasz treść message'a z dala od testu, powinno być tak:

   Kopiuj

   @Test
   void shouldReturnErrorMessageForExistUsername() {
       assertErrorMessage(() -> registrationService.register(
           RegistrationHelper.createExistUsernameRequest()), "Your username is duplicated");
   }
   

   No chyba że message'e są dynamiczne? Ale widzę że nie są, a skoro tak to powinny być blisko testu, bo message nie jest szczegółem implementacyjnym. Przecież cały ten test jest po to żeby sprawdzić message, więc niepotrzebnie chowasz ten message gdzieś dalej.

5. Nie potrzebnie masz osobno testy wyjątku i message'a, moim zdaniem powinieneś mieć to w jednym teście.

   Kopiuj

    @Test
    void shouldThrowExceptionForInvalidEmailFormat() {
      assertException(
        () -> registration.register(RegistrationRequests.invalidEmail()),
        ValidationException.class,
        "You supplied invalid e-mail address"
      );
   }
   

6. Trochę nie kumam czemu masz tyle niepotrzebnych słów w kodzie. Czemu .createInvalidEmailRequest() zamiast .invalidRequest()? Czemu registrationService zamiast registration? Czemu shouldThrowExceptionForInvalidUsernameFormat() zamiast shouldThrowForInvalidUsername()?

A co do Twojej klasy, nie wiem czy ja bym nie zrobił tak:

public class RegistrationRequests {
    private final password = "testPassword@123";

    public RegisterRequest register = new RegisterRequest("user", "mail@mail", password);
    public LoginRequest login = new LoginRequest("user", password);

    public RegisterRequest invalidUsername =  new RegisterRequest("invalid$", "mail@mail", password);
    public RegisterRequest invalidPassword = new RegisterRequest("user", "mail@mail", "invalid");
    public RegisterRequest invalidEmail = new RegisterRequest("user", "invalid", password);

    public RegisterRequest existingUsername = new RegisterRequest("duplicate", "mail@mail", password);
    public RegisterRequest existingEmail = new RegisterRequest("user", "duplicate@mail", password);

    public RegisterRequest register(String username) {
        return new RegisterRequest(username, "mail@mail", password);
    }
}

O ile lepiej się to czyta?

Dane użyte w teście powinny być minimum potrzebne do wytworzenia warunkó testowych, jeśli string invalid$ wystarczy żeby test przeszedł - to taki string dodaj. Nie musisz z niego robić invalidUsername$Format. Dane nie mówią za siebie. Jeśli mail@mail wystarczy żeby przeszły testy, to takie powinny być.

Riddle napisał(a):

Kopiuj

6. Trochę nie kumam czemu masz tyle niepotrzebnych słów w kodzie. Czemu `.createInvalidEmailRequest()` zamiast `.invalidRequest()`? Czemu `registrationService` zamiast `registration`? Czemu `shouldThrowExceptionForInvalidUsernameFormat()` zamiast `shouldThrowForInvalidUsername()`?

Tak się nauczyłem, nie łatwo będzie zmienić ten nawyk.

Riddle napisał(a):

Kopiuj

public class RegistrationRequests {
    private final password = "testPassword@123";

    public RegisterRequest register = new RegisterRequest("user", "mail@mail", password);
    public LoginRequest login = new LoginRequest("user", password);

    public RegisterRequest invalidUsername =  new RegisterRequest("invalid$", "mail@mail", password);
    public RegisterRequest invalidPassword = new RegisterRequest("user", "mail@mail", "invalid");
    public RegisterRequest invalidEmail = new RegisterRequest("user", "invalid", password);

    public RegisterRequest existingUsername = new RegisterRequest("duplicate", "mail@mail", password);
    public RegisterRequest existingEmail = new RegisterRequest("user", "duplicate@mail", password);

    public RegisterRequest register(String username) {
        return new RegisterRequest(username, "mail@mail", password);
    }
}

O ile lepiej się to czyta?

No zdecydowanie lepiej to wygląda.

Riddle napisał(a):

Dane nie mówią za siebie

A ja do tej pory robiłem na odwrót :)

Ornstein napisał(a):

Riddle napisał(a):

Kopiuj

6. Trochę nie kumam czemu masz tyle niepotrzebnych słów w kodzie. Czemu `.createInvalidEmailRequest()` zamiast `.invalidRequest()`? Czemu `registrationService` zamiast `registration`? Czemu `shouldThrowExceptionForInvalidUsernameFormat()` zamiast `shouldThrowForInvalidUsername()`?

Tak się nauczyłem, nie łatwo będzie zmienić ten nawyk.

Po prostu postaraj się zwrócić uwagę na to które słowa są istotne a które nie - i te które nie są, po prostu je wywal.

Nie staraj się wpasować w jakiś "kanon", że jak funkcja robi new, to się zaraz musi nazywać create.

Riddle napisał(a):

Dane nie mówią za siebie

A ja do tej pory robiłem na odwrót :)

Ja rozumiem - chcesz wsadzić do testu dane które mają wyglądem przypominać te, które chciałbyś żeby userzy przekazywali - tylko że tak to nie działa. Userzy wpiszą co im się podoba. Więc nie warto próbować dodawać komplikacji do stringa, które nie są konieczne pod test.

Riddle napisał(a):

Ja rozumiem - chcesz wsadzić do testu dane które mają wyglądem przypominać te, które chciałbyś żeby userzy przekazywali - tylko że tak to nie działa. Userzy wpiszą co im się podoba.

Będę pamiętał.

Przepraszam, że się wpierdzielam do działu Java, ale te nazwy przypadków testowych jak np. shouldUserExist - co to niby znaczy? Kiedy on ma istnieć? Zgodnie z nazwą tego przypadku użytkownik powinien, czyli wcale nie musi istnieć. A więc nie trzeba robić asercji na bazie - wystarczy Assert.isTrue(true), aby spełnić test.

W przypadkach testowych opisujemy fakty, które istnieją w danym kontekście should to nie jest fakt, a shouldUserExist nie zawiera żadnego kontekstu.
Jest bardzo wiele konwencji jak to robić poprawnie, ja bym zaczął np. od czegoś takiego:
WhenValidUserIsRegistered_UserExistInDatabase
WhenMissingUserName_ReturnsInvalidDataMessage
itd., itp.

Test powinien być tak mały żeby w razie wypierdzielenia się było na pierwszy rzut oka widać co i dlaczego się wypierdzieliło.

W twoim przykładzie (brzydkim) mając 4 asserty musisz kminić który z nich się wypierdzielił, to źle.

Roman Mokrzan napisał(a):

Test powinien być tak mały żeby w razie wypierdzielenia się było na pierwszy rzut oka widać co i dlaczego się wypierdzieliło.

W twoim przykładzie (brzydkim) mając 4 asserty musisz kminić który z nich się wypierdzielił, to źle.

Tak, i na to masz dwa rozwiązania:

• Albo podzielić je na testy osobne i nadać im odpowiednie nazwy, jeśli ma to sens
• Albo nałożyć swoją abstrakcję na asercje, tak żeby ich fail pokazywał bardzo dobrą wiadomość razem z diffem, tak żeby było widać co się zepsuło.

Roman Mokrzan napisał(a):

Test powinien być tak mały żeby w razie wypierdzielenia się było na pierwszy rzut oka widać co i dlaczego się wypierdzieliło.

W twoim przykładzie (brzydkim) mając 4 asserty musisz kminić który z nich się wypierdzielił, to źle.

Też kiedyś w to wierzyłem. Tym niemniej:
jak test się wywala - to w 99.9% przypadkach i tak musze zobaczyć stacktrace, stdout itp, żeby mieć pomysł dlaczego się wypierdzieliło.
Jak już mam ten stacktrace to wiem też, który assert się wywalił....

Może ta reguła (jedna asercja na test) pochodzi z czasów jeszcze gorszych frameworków do testów?
Może nie można było łatwo kliknąć w stacktrace, może assercje nie miały komunikatów.... itp.

jarekr000000 napisał(a):

jak test się wywala - to w 99.9% przypadkach i tak musze zobaczyć stacktrace, stdout itp, żeby mieć pomysł dlaczego się wypierdzieliło.
Jak już mam ten stacktrace to wiem też, który assert się wywalił....

Popraw swoje asercje tak żeby wyświetlały ładny message i diffa.

jarekr000000 napisał(a):

Może ta reguła (jedna asercja na test) pochodzi z czasów jeszcze gorszych frameworków do testów?
Może nie można było łatwo kliknąć w stacktrace, może assercje nie miały komunikatów.... itp.

To nie chodziło nigdy o frameworki, tylko o to żeby odejść od tzn. "scenario testing", w których jeden test testuję listę kroków (np. 10 kroków żeby coś kupić, np rejestracja, logowanie, dodanie metody płatności, znalezienie przedmiotu, dodanie go do koszyka, przejście do checkout, zapłacanie, odebranie potwierdzenia). To był wtedy "cały scenariusz", i albo się udał albo nie. Niestety takie rozwiązania były słabe w znajdowaniu problemów, więc powstała idea testów, które testują każdy krok z osobna (wtedy to się nazywało testy jednostkowe, teraz to słowo ma inne znaczenie), tak że każdy z tych kroków można było przetestować osobno (osobny test na rejestracje, osobny na logowanie, osobny na dodanie metody płatności, etc.), ale nadal powinny pokryć cały ten scenariusz - tylko że każdy krok z osobna (taki krok to była jednostka). Zasada jedna asercja per test miała do tego prowadzić. Teraz słowo "test jednostkowy" jest używany w innym znaczeniu.

Różnica pomiędzy takimi podejściami jest bardzo jasna:

Te testy testują to samo, tylko że w drugim podejściu dużo dokładniej widać co się zepsuło. W drugim podejściu, każdy z testów register,login, add payment method można uruchomić niezależnie od siebie w dowolnej kolejności.

Jak chcę przetestować sobie "user scenario" o nazwie "purchase by google pay", to mogę odpalić wszystkie testy, potem widzę że nie działa np "search product", i potem mogę odpalać tylko ten jeden test, bez przywiązania do pozostałych. To była poczatkowa idea za testami jednostkowymi, tylko niestety się nie przyjęła i teraz to słowo znaczy coś innego.