Czy w testach kontrolera dla endpointu logout powinienem weryfikować usunięcie sesji?

Cześć,

Posiadam klasę SessionDeletionImpl, której zadaniem jest usuwanie sesji. Napisałem testy do niej, w których sprawdzam, czy sesja jest prawidłowo usuwana. Tej klasy używam w kontrolerze LogoutController - w momencie wylogowania usuwam sesję. Czy w testach kontrolera, powinienem napisać test, który sprawdza, czy po wylogowaniu sesja rzeczywiście zostaje usunięta? - mam tutaj na myśli test, który strzela do bazy, i sprawdza czy sesja istnieje. Czy wystarczy ten test klasy SessionDeletionImpl?

Myślałem o czymś takim:

 @Test
    void whenLogoutSuccess_DeleteSession() throws Exception {
        Cookie sessionCookie = authHelper.loginRegularUser();

        String sessionId = tokenHasher.generateHashedToken(sessionCookie.getValue());
        
        assertThat(session.sessionExist(sessionId)).isTrue();

        mockMvc.perform(post("/api/v1/users/logout").cookie(sessionCookie))
                .andExpect(status().isOk());

        assertThat(session.sessionExist(sessionId)).isFalse();
    }

Dzięki za pomoc

Tak, możesz.

Aczkolwiek ten test też ma szczegóły implementacyjne niepotrzebnie.

@Test
void noSessionAfterLogout() throws Exception {
    Cookie sessionCookie = auth.loginRegularUser();
    post(sessionCookie, "/api/v1/users/logout");
    assertThat(session.sessionExist(sessionId(sessionCookie))).isFalse();
}

Zamiast strzelać do bazy ja bym po prostu spróbował strzelić do api ze starym ciastkiem i upewnił się że dostaję 401.
Pisz testy tak żeby zielone wystarczyły żeby pchnąć builda na produkcję w piątek o 16:55.
Swoją drogą w czasach JWT to w ogóle rzadkie że sesja tak znika

Jak mówi @obscurity niżej, możesz albo sprawdzić bezpośrednio w repo, albo strzelić pod endpoint i sprawdzić 401. Z punktu widzenia testu, to nie ma znaczenia, bo testujesz endpoint wylogowania, a nie wylogowanie. Z punktu widzenia tego testu który piszesz, sprawdzenie czy nastąpiło wylogowanie powinno być schowane w asercji, i test nie powinien wiedzieć czy sprawdzenie jest przez repo czy przez endpoint.

Natomiast tego czy wybrać sprawdzenie przez repo czy przez endpoint, to nie ma jednej "dobrej odpowiedzi", bo w jednym i drugim przypadku masz coupling na coś.

• Jeśli testujesz używając repo, to dodajesz coupling na szczegół implementacyjny, jakim jest repo - to sprawia że testy są rigid, i znaczy że refaktor repo będzie się równał edycji asercji.
• Jeśli testujesz używając endpointu, to dodajesz coupling na interfejs http logowania, i to sprawia że testy są fragile, czyli zmiana endpointa może sprawić że testy przestanie wykrywać buga, i będzie wymagał pamiętania, żeby poprawić asercję.

Twój wybór.

Riddle napisał(a):

Z punktu widzenia testu, to nie ma znaczenia, bo testujesz endpoint wylogowania, a nie wylogowanie.

A co masz na myśli, pisząc wylogowanie? To powinienem napisać jeszcze jakiś test w osobnej klasie, który sprawdza czy to wszystko działa? Czy chodziło Ci o logikę biznesową?

Ornstein napisał(a):

Riddle napisał(a):

Z punktu widzenia testu, to nie ma znaczenia, bo testujesz endpoint wylogowania, a nie wylogowanie.

A co masz na myśli, pisząc wylogowanie?

Próbowałem powiedzieć, że z punktu widzenia testu, to nie ma znaczenia czy asercja sprawdzi wylogowanie przez repo czy przez 401kę.

Ornstein napisał(a):

To powinienem napisać jeszcze jakiś test w osobnej klasie, który sprawdza czy to wszystko działa?

Jeśli masz tylko jeden sposób żeby kogoś wylogować (ten endpoint), i nie wymaga to więcej kombinowania niż po prostu przycisk "wyloguj", to wystarczy ten jeden.

@Riddle
Zamiast strzelać do bazy i weryfikować, czy sesja została usunięta, zmieniłem na strzał do endpointu i sprawdzenie 401. Jak oceniasz?

Test:

   @Test
    void whenLogoutSuccess_SecuredRequestUnauthorized() throws Exception {
        Cookie sessionCookie = authHelper.loginRegularUser();
        authHelper.logoutUser(sessionCookie);

        mockMvc.perform(get("/api/v1/users/1").cookie(sessionCookie))
                .andExpect(status().isUnauthorized());
    }

Helper:

  public void logoutUser(Cookie sessionCookie) {
        try {
            mockMvc.perform(post("/api/v1/users/logout").cookie(sessionCookie))
                    .andExpect(status().isOk());
        } catch (Exception e) {
            throw new RuntimeException("Logout failed " + e.getMessage());
        }
    }

    public Cookie loginAndGetCookie(LoginRequest request) {
        try {
            MvcResult mvcResult = mockMvc.perform(post("/api/v1/users/login")
                            .contentType(MediaType.APPLICATION_JSON)
                            .content(asJsonString(request)))
                    .andExpect(status().isOk())
                    .andReturn();

            return Arrays.stream(mvcResult.getResponse().getCookies())
                    .filter(cookie -> "session_id".equals(cookie.getName()))
                    .findFirst()
                    .orElseThrow(() -> new AssertionError("Session cookie not found"));
        } catch (Exception e) {
            throw new RuntimeException("Failed to login for user " + request.usernameOrEmail());
        }
    }

Nadal test wie o tym jak to sprawdzasz.

Ja bym napisał tak:

@Test
void whenLogoutSuccess_SecuredRequestUnauthorized() throws Exception {
    Cookie sessionCookie = authHelper.loginRegularUser();
    authHelper.logoutUser(sessionCookie);
    assertTrue(isLoggedOut(sessionCookie));;
}

i ten check powinien być w isLoggedOut() ten strzał pod endpoint.

PS: Nie ma po co robić .andExpected(), ja bym po prostu odczytał status return status() == 401.
PS2: Nie wiem czy jest sens nazywać coś "helper" w teście. Ja bym to nazwał po prostu auth.

@Riddle
Miałbym jeszcze takie pytanie. Kiedy tworzę takie klasy pomocnicze do testów, tak ja ta poniżej, czy mogę zrobić tak, że nie będę jej oznaczał jako @Component, tylko zainicjuję ją w klasie testowej, tworząc nowy obiekt i przekazując MockMvc? Czy takie podejście jest "dobre"?

Tak by to wyglądało.

Helper:

public class AuthenticationTestHelper {

    private final MockMvc mockMvc;

    public AuthenticationTestHelper(MockMvc mockMvc) {
      this.mockMvc = mockMvc;
    }

    public void logoutUser(Cookie sessionCookie) {
    }

    public Cookie loginAndGetCookie(LoginRequest request) {
    }
}

Klasa testowa:

@AutoConfigureMockMvc
public class LogoutControllerTest extends BaseIT {

    @Autowired
    private MockMvc mockMvc;
    private AuthenticationTestHelper auth;

    @Autowired
    private SqlDatabaseInitializer initializer;
    @Autowired
    private SqlDatabaseCleaner cleaner;

    @BeforeEach
    void setUp() throws Exception {
        auth = new AuthenticationTestHelper(mockMvc);
        initializer.setup("data_sql/test-setup.sql");
    }

    @AfterEach
    void cleanUp() {
        cleaner.clean();
    }

// testy

Ornstein napisał(a):

@Riddle
Miałbym jeszcze takie pytanie. Kiedy tworzę takie klasy pomocnicze do testów, tak ja ta poniżej, czy mogę zrobić tak, że nie będę jej oznaczał jako @Component, tylko zainicjuję ją w klasie testowej, tworząc nowy obiekt i przekazując MockMvc? Czy takie podejście jest "dobre"?

No pewnie.

Po co masz dodawać coupling na Springa.

Najlepszy test byłby taki, który w ogóle nie wie o springu:

import org.junit.jupiter.api.Test;

public class LogoutControllerTest {
    private TestHelper auth;

    @BeforeEach
    void before() {
        auth = new TestHelper("data_sql/test-setup.sql");
    }

    @AfterEach
    void after() {
        auth.clean();
    }
}

Spring, jak i wiele innych frameworków, mają przykłady w dokumentacji które zalecają silny coupling z tym frameworkiem. To nie jest dobre podejście. Jak dodajesz adnotacje, rozszerzasz klasy z frameworka, to przywiązujesz swoje testy do tego frameworka, przez co ciężej je potem utrzymać.