Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beds 982/login tests #1168

Merged
merged 1 commit into from
Dec 20, 2024
Merged

Beds 982/login tests #1168

merged 1 commit into from
Dec 20, 2024

Conversation

tanya-bitfly
Copy link

No description provided.

Copy link

cloudflare-workers-and-pages bot commented Nov 27, 2024

Deploying beaconchain with  Cloudflare Pages  Cloudflare Pages

Latest commit: 73e275b
Status: ✅  Deploy successful!
Preview URL: https://70820815.beaconchain.pages.dev
Branch Preview URL: https://beds-982-login-tests.beaconchain.pages.dev

View logs

Copy link
Contributor

@marcel-bitfly marcel-bitfly left a comment

Choose a reason for hiding this comment

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

good work so far, thx for making such good progress in such a short amount of time 💪

frontend/.gitignore Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

reminder:

package-lock.json has to be removed and recreated before merging, as other parts might have changed in the meantime

frontend/tests/page-object/base.page.ts Outdated Show resolved Hide resolved
frontend/tests/page-object/dashboard.page.ts Outdated Show resolved Hide resolved
backButton: (page: Page) => page.locator("text=Back"),
continueNetworkButton: (page: Page) => page.getByRole("button", { name: "Continue" }),

clickContinue: async (page: Page) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

would be nice if we use the most generic form of our app behaviour, which does not rely on the users device.
Do I really click on a mobile device?

What if I tell alexa or siri that I want to do something specific....if tests could be reduced to the actual behavioural pattern we will have the most resilient tests

Suggested change
clickContinue: async (page: Page) => {
continue: async (page: Page) => {

errorPassword: (page: Page) => page.locator(".p-error").nth(1),
toastMessage: (page: Page) => page.locator(".p-toast-message-error"),

login: async (page: Page, userEmail: string, userPassword: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

haha an alltime classic...I guess we should change our button in frontend as well, shouldn't we?

https://ux.stackexchange.com/questions/1080/using-sign-in-vs-using-log-in

frontend/package.json Show resolved Hide resolved

export const BasePage = {
goto: async (page: Page, endpoint: string) => {
await page.goto("https://v2-staging-mainnet.beaconcha.in" + endpoint);
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this should suffice, if we set the base url in the config

Suggested change
await page.goto("https://v2-staging-mainnet.beaconcha.in" + endpoint);
await page.goto(endpoint);

frontend/tests/playwright.config.ts Outdated Show resolved Hide resolved
Comment on lines 3 to 10
import dotenv from 'dotenv';
import path from 'path';
import { fileURLToPath } from 'url';

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);

dotenv.config({ path: path.resolve(__dirname, './tests/.env') });
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import dotenv from 'dotenv';
import path from 'path';
import { fileURLToPath } from 'url';
const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
dotenv.config({ path: path.resolve(__dirname, './tests/.env') });
import dotenv from 'dotenv'
dotenv.config({ path: path.resolve(process.cwd(), '.env') })

this should work

@marcel-bitfly
Copy link
Contributor

np: commit msgs

Before merging we should clean up all commit messages locally I can help with interactive rebasing

@tanya-bitfly tanya-bitfly force-pushed the BEDS-982/login-tests branch 2 times, most recently from eb32c94 to 4105f1f Compare November 29, 2024 13:31
frontend/package.json Outdated Show resolved Hide resolved
@marcel-bitfly
Copy link
Contributor

plz do a npm run lint --fix also configure your ide correctlyas there are a lot offormatting` issues

frontend/tests/specs/auth-user/login.spec.ts Outdated Show resolved Hide resolved
frontend/tests/page-object/login.page.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,8 @@
import type { Page } from '../utils/helpers'

export const BasePage = {
Copy link
Contributor

Choose a reason for hiding this comment

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

this file was not referenced anymore

frontend/tests/utils/helpers.ts Outdated Show resolved Hide resolved
@marcel-bitfly marcel-bitfly merged commit 6fe94fc into staging Dec 20, 2024
3 checks passed
@marcel-bitfly marcel-bitfly deleted the BEDS-982/login-tests branch December 20, 2024 12:45
@marcel-bitfly
Copy link
Contributor

@tanya-bitfly good work....we are on the best way to improve our frontend resilience thx again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants