Add POM for the E2E tests and update all tests #1259

DanielTakev · 2022-12-16T10:46:52Z

Closes #{#1188, #1194 }

Motivation and context

This change is important because we need stable E2E tests and reusable code.

What we have:

Page Object Model
Separated dependencies into the E2E folder
Reusable methods which work both with EN and BG language versions
Test reports + screenshots on failure and other details
Additional: removed Cyrillic symbol (e) from a few files (ex.: public/locales/bg/common.json)

If we want to run local E2E tests:

Just change one method in the BeforeAll section of the desired spec and the test will run against localhost

New or updated dependencies:

All Playwright dependencies were updated and moved into folder e2e

TODO:

Check with the developers all CI/CD configurations
Fix all warnings from the pipeline
Update what's left from the previous specs

Added method and configurations Additional refactoring - methods, specs and gitignore Implemented Support page for E2E tests Fixed tests for Support page New changes on the tests

Added method and configurations Additional refactoring - methods, specs and gitignore Implemented Support page for E2E tests Fixed tests for Support page New changes on the tests Fixes on E2E tests

…e-e2e-tests' of https://github.com/podkrepi-bg/frontend into 1194-feature-create-playwright-page-object-model-for-the-e2e-tests

…-model-for-the-e2e-tests

…e-e2e-tests' of https://github.com/podkrepi-bg/frontend into 1194-feature-create-playwright-page-object-model-for-the-e2e-tests

…l-for-the-e2e-tests' of https://github.com/podkrepi-bg/frontend into 1194-feature-create-playwright-page-object-model-for-the-e2e-tests" This reverts commit 3bb8a19, reversing changes made to b18b16b.

github-actions · 2022-12-16T10:47:08Z

✅ Tests will run for this PR. Once they succeed it can be merged.

dimitur2204

Amazing job overall 💯 !
The thing I do not understand is why we need the package.json and .gitignore inside of the e2e folder and not on a global level. Is it so that when it's run in GH Actions we run it inside of the e2e folder and is that what we shoud do?

Also I am concerned on the speed of the tests a little, they've been quite slow running on a single worker before when they were just a few. Now we are going to be waiting a ton for a test suite to run. I don't know what we can do about it though

dimitur2204 · 2022-12-18T11:12:30Z

e2e/.gitignore

+node_modules/
+/playwright/.cache/
+/playwright-report/
+/playwright/.cache/
+/e2e-reports/


maybe these should be in the top level gitignore

I think it won't be a problem to remove this gitignore file (it was auto-generated) and to move on the top level all these folders.
Thanks for the suggestion, I will do it.

Regarding the package.json the situation is more complex. I'm definitely not an expert on such configuration topics, but I discuss that with @kachar and he agreed that it's better to separate all playwright packages into the E2E folder.
In that case will be easier to upgrade the versions, because currently I'm hitting some issues/errors when trying to do it on the main package.json.

As I said, I'm not an expert on this, so I will be grateful to have good discussion about Pros and Cons of such approach.
Thanks overall for your comments, @dimitur2204

I see no problem in having these folders ignored in the current folder .gitignore file

dimitur2204 · 2022-12-18T11:18:35Z

e2e/pages/web-pages/home.page.ts

+    /**
+     * Check if H1 heading is visible by text with timeout
+     * @param {LanguagesEnum} language
+     * @param {string} headingBg
+     * @param {string | null} headingEn
+     */
+    async isH1HeadingVisible(language: LanguagesEnum, headingBg: string, headingEn: string | null): Promise<boolean> {
+        return this.isHeadingVisibleBySelector(this.h1HeadingsSelector, language, headingBg, headingEn);
+    }
+
+    /**
+     * Check if H4 heading is visible by text with timeout
+     * @param {LanguagesEnum} language
+     * @param {string} headingBg
+     * @param {string | null} headingEn
+     */
+    async isH4HeadingVisible(language: LanguagesEnum, headingBg: string, headingEn: string | null): Promise<boolean> {
+        return this.isHeadingVisibleBySelector(this.h4HeadingsSelector, language, headingBg, headingEn);
+    }
+
+    /**
+     * Check if H5 heading is visible by text with timeout
+     * @param {LanguagesEnum} language
+     * @param {string} headingBg
+     * @param {string | null} headingEn
+     */
+    async isH5HeadingVisible(language: LanguagesEnum, headingBg: string, headingEn: string | null): Promise<boolean> {
+        return this.isHeadingVisibleBySelector(this.h5HeadingsSelector, language, headingBg, headingEn);
+    }
+
+    /**
+     * Check if H6 heading is visible by text with timeout
+     * @param {LanguagesEnum} language
+     * @param {string} headingBg
+     * @param {string | null} headingEn
+     */
+    async isH6HeadingVisible(language: LanguagesEnum, headingBg: string, headingEn: string | null): Promise<boolean> {
+        return this.isHeadingVisibleBySelector(this.h6HeadingsSelector, language, headingBg, headingEn);
+    }


These can probably be one function with an option of 'h1'|'h2'|'h3' ...

How and where do you suggest to pass the parameter?
I think it's better to wrap each method as like now, because it is easier when we have less arguments.

e2e/tests/regression/donation-flow/anon-donation-fixed.spec.ts

dimitur2204 · 2022-12-19T16:38:58Z

e2e/tests/regression/donation-flow/donation.spec.ts

+import { expectCopied } from '../../../utils/helpers'
+import { HomePage } from '../../../pages/web-pages/home.page';
+
+// TODO: Refactor this spec


This file is irrelevant at this point I guess

I also think so, but I don't want to remove it yet.

dimitur2204 · 2022-12-19T16:39:29Z

e2e/tests/regression/donation-flow/donation.spec.ts

+  test.describe.skip('user bank transfer donation flow', () => {
+    test('check copied values', async ({ page }) => {
+      // Choose a predefined value from the radio buttons
+      await page.locator('input[value="bank"]').check()
+      expect(await page.locator('text="Копирай"').count()).toBe(4)
+
+      if (page.context().browser()?.browserType().name() === 'chromium') {
+        await page.context().grantPermissions(['clipboard-read', 'clipboard-write'])
+        await page.locator('text="Копирай"').nth(0).click()
+        await expectCopied(page, 'Сдружение Подкрепи БГ')
+        await page.locator('text="Копирай"').nth(1).click()
+        await expectCopied(page, 'Уникредит Булбанк')
+        await page.locator('text="Копирай"').nth(2).click()
+        await expectCopied(page, 'BG66UNCR70001524349032')
+        await page.locator('text="Копирай"').nth(3).click()
+        const reference = await page.locator('p[data-testid="payment-reference-field"]').innerText()
+        await expectCopied(page, reference)
+      }
+    })
+  });


Maybe we should only leave this test running since we don't have it in the new files

Yes, I will review and leave some appropriate scenarios. I just didn't have enough time for all specs.

Additional fix

slavcho · 2022-12-22T13:51:49Z

e2e/data/support-page-tests.data.ts

+
+export const anonDonationTestData = {
+    cardNumber: '4242 4242 4242 4242',
+    cardExpDate: '04 / 24',


can you move the year to say: 42 (👍🌌) ?
We don't want those tests to stop working in 18 months...

slavcho · 2022-12-22T14:14:38Z

e2e/pages/web-pages/campaigns/donation.page.ts

+        if (language === LanguagesEnum.BG) {
+            return this.isStepActiveByLabelText(this.bgSelectAmountSectionText);
+        } else if (language === LanguagesEnum.EN) {
+            return this.isStepActiveByLabelText(this.enSelectAmountSectionText);
+        } else {
+            throw new Error("Language not found!");
+        }


I think those ifs create unnecessary code duplication.
It would be better to have a localization map or even different configuration files per language.
and then just say:

return this.isStepActiveByLabelText(t(lang, 'select-amount-section-text'));

slavcho · 2022-12-22T14:34:51Z

e2e/pages/web-pages/campaigns/campaigns.page.ts

+    private readonly bgMainCampaignsHeading = bgLocalizationCampaigns.campaigns;
+    private readonly enMainCampaignsHeading = enLocalizationCampaigns.campaigns;
+    private readonly bgSupportCauseTodayHeading = bgLocalizationCampaigns.cta['support-cause-today'];
+    private readonly enSupportCauseTodayHeading = enLocalizationCampaigns.cta['support-cause-today'];
+    private readonly bgSupportNowActionButtonText = bgLocalizationCampaigns.cta['support-now'];
+    private readonly enSupportNowActionButtonText = enLocalizationCampaigns.cta['support-now'];


Isn't there a way to use the t() function?
Hardcoding those seems like a lot of code duplication to me.

kachar · 2023-01-16T22:14:48Z

@DanielTakev @dimitur2204 @slavcho Let's move on with this topic and get the initial version merged. We can enhance it once we have the foundation settled in.

What's blocking this PR from getting merged except for the conflict and the failing next.js build?

dimitur2204 · 2023-01-29T16:16:41Z

Closed in favor of #1265

DanielTakev added 8 commits December 15, 2022 23:10

Created basic POM for the Playwright tests - initial commit

7f48285

Added method and configurations Additional refactoring - methods, specs and gitignore Implemented Support page for E2E tests Fixed tests for Support page New changes on the tests

Created basic POM for the Playwright tests - initial commit

b18b16b

Added method and configurations Additional refactoring - methods, specs and gitignore Implemented Support page for E2E tests Fixed tests for Support page New changes on the tests Fixes on E2E tests

Merge branch '1194-feature-create-playwright-page-object-model-for-th…

3bb8a19

…e-e2e-tests' of https://github.com/podkrepi-bg/frontend into 1194-feature-create-playwright-page-object-model-for-the-e2e-tests

Updated paths for the localization files for E2E

0467e36

Merge branch 'master' into 1194-feature-create-playwright-page-object…

26e6314

…-model-for-the-e2e-tests

Merge branch '1194-feature-create-playwright-page-object-model-for-th…

3948470

…e-e2e-tests' of https://github.com/podkrepi-bg/frontend into 1194-feature-create-playwright-page-object-model-for-the-e2e-tests

Updated Readme file

cfe5caa

kachar added the run tests Allows running the tests workflows for forked repos label Dec 16, 2022

github-actions bot removed the run tests Allows running the tests workflows for forked repos label Dec 16, 2022

dimitur2204 reviewed Dec 19, 2022

View reviewed changes

DanielTakev changed the title ~~[DRAFT] Add POM for the E2E tests and update all tests~~ Add POM for the E2E tests and update all tests Dec 21, 2022

Fixed Sonar issues and updated Playwright version

c59609d

Additional fix

DanielTakev force-pushed the master branch from 967405f to c59609d Compare December 21, 2022 23:29

dimitur2204 mentioned this pull request Dec 22, 2022

#1188, #1194 - Add POM for the E2E tests and update all tests #1265

Merged

5 tasks

slavcho requested changes Dec 22, 2022

View reviewed changes

kachar added the run tests Allows running the tests workflows for forked repos label Jan 16, 2023

github-actions bot removed the run tests Allows running the tests workflows for forked repos label Jan 16, 2023

dimitur2204 closed this Jan 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add POM for the E2E tests and update all tests #1259

Add POM for the E2E tests and update all tests #1259

DanielTakev commented Dec 16, 2022 •

edited

Loading

github-actions bot commented Dec 16, 2022 •

edited

Loading

dimitur2204 left a comment

dimitur2204 Dec 18, 2022

DanielTakev Dec 19, 2022

DanielTakev Dec 19, 2022

DanielTakev Dec 19, 2022

kachar Dec 22, 2022

dimitur2204 Dec 18, 2022

DanielTakev Dec 19, 2022

dimitur2204 Dec 19, 2022

DanielTakev Dec 19, 2022

dimitur2204 Dec 19, 2022 •

edited

Loading

DanielTakev Dec 19, 2022

slavcho Dec 22, 2022

slavcho Dec 22, 2022

slavcho Dec 22, 2022

kachar commented Jan 16, 2023

dimitur2204 commented Jan 29, 2023

Add POM for the E2E tests and update all tests #1259

Add POM for the E2E tests and update all tests #1259

Conversation

DanielTakev commented Dec 16, 2022 • edited Loading

Motivation and context

New or updated dependencies:

TODO:

github-actions bot commented Dec 16, 2022 • edited Loading

dimitur2204 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimitur2204 Dec 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kachar commented Jan 16, 2023

dimitur2204 commented Jan 29, 2023

DanielTakev commented Dec 16, 2022 •

edited

Loading

github-actions bot commented Dec 16, 2022 •

edited

Loading

dimitur2204 Dec 19, 2022 •

edited

Loading