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

Feature/playwright e2e tests #1233

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

savilashokworks
Copy link

The login tests were improved although they still need the env values for the user and pass.
The signup tests are being developed.

…locators, pages and specs. The firsts tests about the LOGIN feature are still in progress
…needs to add env variables for user and password
Copy link

vercel bot commented Aug 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
drive-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 18, 2024 2:56pm

…d. The setup file was created in order to log in automatically for tests that require logging in. The setup was configured for chrome, firefox and Safari
Copy link

cloudflare-workers-and-pages bot commented Aug 15, 2024

Deploying drive-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: f0ddcf6
Status:⚡️  Build in progress...

View logs

Copy link
Contributor

@CandelR CandelR 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 @savilashokworks. This is looking good.

Comment on lines 3 to 4
email: 'testingb2b@inxt.com',
password: 'test123.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could make part of the email dynamic, so that each time the signup is tested as well

Copy link
Author

Choose a reason for hiding this comment

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

Ramon!! Yes, it makes perfect sense and I should've done that way! Great, I'll include it in my next commit ;)

import { loginPage } from '../pages/loginPage';
import { staticData } from '../helper/staticData';

const authFile = './tests/specs/playwright/.auth/user.json';
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried to automate the login for each test so you don't have to have the user data in a file?
This may limit some days to do tests, because apart from putting sensitive data of the user in question, it will be filled with test data when uploading files or folders and perform certain actions.
This together with the dynamic email and the sign up, you will be able to test each run with an independent user.
Let me know if I miss anything 😄

Copy link
Author

@savilashokworks savilashokworks Oct 7, 2024

Choose a reason for hiding this comment

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

Let me know if I'm right. You want to log in before each test (no matter if the tests include "log in" tests or not) ❓

In that case, I think I still need the login.setup.ts so as to create the user, log in and save the cookies (that's what it's meant to do). Then I can just ignore the cookies stored and log in in a before hook. I would still need to figure out how to pass the credentials into the tests without revealing data, is that right?

But the setup.ts speeds the whole testing up by using the cookies instead of logging in "manually", that's the benefit of it, but if you're ok with logging in before each test, it can be done, definitely. Unless you want to get rid of the setup.ts D:

tests/locators/driveLocators.ts Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Sep 23, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
6.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@@ -0,0 +1 @@
{"email":"Art.Goyette77@yahoo.com","password":"whSneqH707MPwbp"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Be careful when exposing sensitive data such as username/password. The user has to be dynamic in every test.
With cypress we wrote the generated user and password dynamically in a json so that after registering the user, the other tests could use the login and re-run the test

await this.page.locator('body').click({ button: 'right' });
await this.page.waitForTimeout(500);
//await expect(this.rightClickOnBodyModal).toBeVisible()
Copy link
Contributor

Choose a reason for hiding this comment

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

forgot to remove this commeted line.

const createAccountTitle = await this.createAccounTitle.textContent();
await expect(this.createAccounTitle).toBeVisible();
const createAccountHeader = this.createAccounTitle.textContent();
//console.log(createAccountHeader)
Copy link
Contributor

Choose a reason for hiding this comment

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

forgot to remove this commented log.

@@ -27,7 +29,7 @@ export default defineConfig({
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
use: {
/* Base URL to use in actions like `await page.goto('/')`. */
baseURL: 'https://staging.drive.internxt.com/login',
baseURL: 'https://drive.internxt.com/login',
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to run the test with the current branch in a local environment, not pointing to the production frontend, the backend could be the production one but not the frontend.
If you point to the production frontend you are not testing the actual changes.

Comment on lines 8 to 10
passwordRecoveryLinkURL: 'https://drive.internxt.com/recovery-link/',
signUpURL: 'https://drive.internxt.com/new',
driveURL: 'https://staging.drive.internxt.com/',
driveURL: 'https://drive.internxt.com/',
Copy link
Contributor

Choose a reason for hiding this comment

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

Better add a constant with the baseURL and reuse it everywhere, otherwise it's going to be a hassle if you have to add any changes.

await expect(newPage).toHaveURL(/.*login/);

const endpointPromise = newPage.waitForResponse('https://drive.internxt.com/api/access');
//const endpointPromise = newPage.waitForResponse('https://drive.internxt.com/api/access');
const endpointPromise = newPage.waitForResponse('https://gateway.internxt.com/drive-old/access');
Copy link
Contributor

Choose a reason for hiding this comment

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

Add all the urls to constants

@@ -36,10 +36,11 @@ setup('Creating new user and logging in', async ({ browser }) => {

const loginpage = new loginPage(newPage);

await newPage.goto('https://staging.drive.internxt.com/login');
await newPage.goto('https://drive.internxt.com/login');
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, use a constnat for urls


await page.goto('https://staging.drive.internxt.com/new');
await page.goto('https://drive.internxt.com/new');
Copy link
Contributor

Choose a reason for hiding this comment

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

remember to use the constnat for urls and point to local environment

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

Successfully merging this pull request may close these issues.

3 participants