Add new test for open a page and screenshot function #1
Conversation
| ).toHaveText(ApplicationTexts.registrationPage.registrationButtonLabel); | ||
| }); | ||
|
|
||
| test("verify registration form using screenshot", async ({ page }) => { |
There was a problem hiding this comment.
Samostatný screenshot by neměl být test. Můžeš ho ale klidně dát do testu verify registration form is displayed correctly
|
|
||
| test.describe("Registration Page is Correctly Displayed", async () => { | ||
| test.beforeEach(async ({ page }) => { | ||
| const loginPage = new LoginPage(page); |
There was a problem hiding this comment.
Pro účely tohoto zadání není potřeba více page object modelů než jeden, a to RegistrationPage. Do něj se dá vložit metoda pro otevření stránky open()
| }); | ||
| }); | ||
|
|
||
| test.describe("User Registration - Valid and Invalid Credentials", () => { |
There was a problem hiding this comment.
Myslím že není potřeba mít dva test.describe(). Jeden test.describe() by stačil.
| let testEmail; | ||
|
|
||
| test.beforeAll(async () => { | ||
| testEmail = `testuserCzechitas+${Date.now()}@gmail.com`; |
There was a problem hiding this comment.
testEmail generuje vždy unikátní e-mail, protože Date.now() negeneruje jen datum, ale i čas. To znamená, že test should not register with existing email vždy failne.
There was a problem hiding this comment.
Pro účely testu should not register with existing email by stačil statický, již existující e-mail (třeba da-app.admin@czechitas.cz), generováním dynamického e-mailu se vystavujeme flakiness. V takovém případě by ani nemusel být v test.beforeAll(), ale cením snahu beforeAll() nějak zakomponovat.
| test("should not register with existing email", async ({ page }) => { | ||
| const registrationPage = new RegistrationPage(page); | ||
| await registrationPage.register(userFullName, testEmail, password); | ||
| await expect(registrationPage.toast).toHaveText( |
There was a problem hiding this comment.
Assert toHaveText() je fajn, ale mnohem důležitější assert je .isVisible() - zajišťuje totiž, že se element skutečně zobrazuje v DOMu a není třeba hidden nebo překrytý jinou komponentu. Bylo by fajn assertnout i text i visibilitu.
| ); | ||
| }); | ||
|
|
||
| test("should not register with invalid password - only digits", async ({ |
There was a problem hiding this comment.
formátování:
test("should not register with invalid password - only digits", async ({ page }) => {
| @@ -0,0 +1,30 @@ | |||
| export class AppPage { | |||
There was a problem hiding this comment.
Stejně jako LoginPage, ani AppPage object model tu úplně nemusí být. Vzniká totiž tzv. dead code - nevyužitý kód - funkce getToastMessage(), logout() a getCurrentUser() se nikde v rámci /test/homework.spec.js nepoužívají.
|
|
||
| import { AppPage } from "./app.page"; | ||
|
|
||
| export class RegistrationPage extends AppPage { |
There was a problem hiding this comment.
Jak jsem zmínila: RegistrationPage jako page object model dokáže pobrat všechny elementy, lokátory i funkce potřebné k pokrytí stránky registrace. LoginPage a AppPage nejsou potřeba. Struktura s víc page object modely dává smysl u testů, kde testujeme více stránek - tento úkol je ale pouze o testování stránky registrace.
|
Code review done. Merge request bohužel nebudeme moci approvnout, protože je to merge request do našeho czechitas repozitáře, namísto forku na tvém githubu. Velmi chválím použití fixtures! Do fixtures by se daly přidat jinak i password a username test data. Chválím snahu použít test.describe, test.beforeAll, test.beforeEach, akorát někdy méně je více a v tomto rozsahu nejsou úplně potřeba. Velmi cením použití page object modelu - to bylo v zadání velmi optional, ale stačil by jen jeden page object model a vyšperkovat. Cením také snahu o dynamické testovací data (testEmail), ale zrovna v tomto případě to nemá ten efekt - nicméně určitě budou případy, kde budou dynamické testovací data potřeba! Testy jsou spustitelné (kromě test casu s již existujícím e-mailem), pojmenovávání proměnných dává smysl a je fajn, rozsah test casů je dobrý a splňuje zadání, asserty jsou přítomny ale mohlo by jich být více (hlavně isVisible() je alfa omega). Jsou tu trošku chybky ve formátování, ale s tím dokáže velmi efektivně pomoci eslint nebo jiný helper, ten to pak dělá automaticky. Celkově je to fajn, trošku péče by to ještě potřebovalo, ale určitě se s tím dá pracovat a není to vůbec špatné. Děkuji! |
As testEmail must be shared across tests, ensure only one worker runs tests (npx playwright test --workers=1)
No description provided.