Skip to content

Conversation

@maciejkrol18
Copy link
Member

@maciejkrol18 maciejkrol18 commented Jul 24, 2025

Dodaje testy E2E do części aplikacji z szablonami mailów. Ten branch, https://github.com/Solvro/web-eventownik-v2/tree/test/emails, został stworzony z https://github.com/Solvro/web-eventownik-v2/tree/feat/mail-tags i docelowo ma być w niego zmergowany - zrobiłem tak żeby od razu móc już napisać testy do znaczników ale równocześnie nie robić tego w tej samej PR'ce żeby był porządek. Sam branch ze znacznikami i tak nie zostanie zmergowany aż nie zostanie ogarnięte ich handlowanie po stronie backendu.

Lista testów do napisania

  • Formularz dodawania szablonu
    • Wybór wyzwalaczy
    • Wpisywanie tytułu i zawartości
    • Zapisywanie nowego szablonu
  • Usuwanie szablonów (zrobione ale niezbyt prawidłowo na tę chwilę)
  • Edycja szablonów
  • Edytor WYSIWYG
    • Ogólne
      • Wpisywanie tekstu
      • Usuwanie tekstu
      • Tworzenie nowej linii
      • Wklejanie tekstu
      • Wstawianie obrazków
    • Formatowanie (zarówno przez markdown w tekście i przez menu)
      • Pogrubienie
      • Tekst pochyły
      • Kod (czcionka mono)
      • Nagłówek pierwszego stopnia
      • Nagłówek drugiego stopnia
      • Nagłówek trzeciego stopnia
      • Wyrównanie do lewej
      • Wyrównanie do środka
      • Wyrównanie do prawej
      • Justowanie
    • Znaczniki
      • Wstawianie znaczników z menu (czy po kliknięciu przycisku w tekście wpisuje się slash)
      • Widoczność menu z listą po wpisaniu "/" (czy po wpisaniu slasha wyskakuje lista znaczników)
      • Wyszukiwanie w liście znaczników (kilka wyników, jeden wynik, brak wyników)
      • Widoczność znaczników w edytorze (czy po wybraniu znacznika pojawia się on w tekście)

Important

Add E2E tests for email templates, configure Playwright, and update components for testability and image handling.

  • E2E Tests:
    • Add tests in emails.spec.ts for creating, editing, and deleting email templates.
    • Test WYSIWYG editor features: text input, formatting, image insertion, and tag handling.
    • Use auth.setup.ts for authentication setup in tests.
  • Configuration:
    • Add playwright.config.ts to configure Playwright for E2E tests.
    • Update package.json to include Playwright and Testing Library dependencies.
  • Components:
    • Update EditorMenuBar to handle image uploads and show error for unsupported types.
    • Add role="textbox" and data-testid="editor" to WysiwygEditor for testability.
    • Add role="menu" to TagsList for accessibility.

This description was created by Ellipsis for 84cfc7a. You can customize this summary. It will automatically update as commits are pushed.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 24, 2025
@maciejkrol18 maciejkrol18 changed the title test: add integration tests for email wysiwyg editor test: add e2e tests for email templates Jul 25, 2025
@maciejkrol18 maciejkrol18 marked this pull request as ready for review August 2, 2025 17:24
});
});

// TODO: Refactor the whole creating and deleting mechanism
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests for creating and deleting templates depend on running in a specific sequence. Refactor test state to isolate cases and avoid inter-test dependencies.

Suggested change
// TODO: Refactor the whole creating and deleting mechanism
// TODO: Refactor test state to isolate cases and avoid inter-test dependencies for creating and deleting templates.

@maciejkrol18 maciejkrol18 marked this pull request as draft August 14, 2025 17:22
@Antoni-Czaplicki Antoni-Czaplicki requested a review from Rei-x August 18, 2025 10:51
Rei-x
Rei-x previously approved these changes Aug 20, 2025
Copy link
Member

@Rei-x Rei-x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

widać, że się bardzo postarałeś - użycie POMa oraz poprawny setup autha, niewiele osób zrobiło obie te rzeczy 😭

testy same w sobie są bardzo ładne - testują mają konkretną część tego komponentu. Duży minus jest taki, że testy muszą być puszczane w serialu przez co ich debugowalność spada do zera (zobaczysz o czym mówię jak pierwszy raz zaczną failować). Idealnie każdy test powinien setupować swoje środowisko od zera - dzięki temu też mogą być odpalane na wielu wątkach i szybciej lecieć.

poza tym do tworzenia użytkowników testowych polecam wybłagać backend o jakiś endpoint albo samemu tam wskoczyć

});
});

test.describe("Editor", () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oj oj, to jest słaby test - testy e2e powinny być maksymalnie małe i testować jedną konkretną rzecz - ten tutaj testuje kilkanaście różnych rzeczy w jednym teście 😭

każdy test-case powinien być niezależny od siebie i odpalanie ich na serialu to proszenie się o kłopoty - jeśli chcesz jakiejś ładnej abstrakcji by podzielić jeden duży test to masz test.step. Jeśli się BARDZO nie da rozdzielić tych test case'ów to okej, ale przemyślałbym w takim razie czy nie trzeba pozmieniać kodu - debugowanie tych testów to będzie koszmar, bo wszystko jest zależne od siebie 😭

idealnie powinieneś móc odpalić wszystkie testy in parallel i powinny one przejść bez problemu - a po zfailowaniu powinny usunąć swoje dany z api

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Przeniosłem cały suite z edytorem do osobnego pliku, mam nadzieje że o to chodziło. Plus na tę chwilę postanowiłem usunąć test z usuwaniem żeby mogło działać parallel. Taki bandaid fix ale bez specjalnych endpointów z backendu trochę by było paprania się w tym

@Antoni-Czaplicki Antoni-Czaplicki added the tests Tests for the code label Aug 23, 2025
Base automatically changed from feat/mail-tags to main November 10, 2025 12:50
@maciejkrol18 maciejkrol18 dismissed Rei-x’s stale review November 10, 2025 12:50

The base branch was changed.

@Antoni-Czaplicki Antoni-Czaplicki force-pushed the main branch 9 times, most recently from ac64c27 to f99d90f Compare November 24, 2025 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XXL tests Tests for the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants