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

test: playwright boilerplate #65

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,9 @@ yarn-error.log*
next-env.d.ts

.idea/
node_modules/
/test-results/
/playwright-report/
/playwright/.auth
/blob-report/
/playwright/.cache/
Binary file modified bun.lockb
Binary file not shown.
53 changes: 53 additions & 0 deletions e2e/auth.setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { test as setup } from '@playwright/test';
import path from 'path';
import { signInWithGitHub } from '@/e2e/utils/auth';

const authFile = path.join(__dirname, '../playwright/.auth/user.json');

const github = {
email: process.env.GITHUB_USER_EMAIL ?? '',
password: process.env.GITHUB_USER_PASSWORD ?? '',
};

setup('authenticate', async ({ page }) => {
setup.setTimeout(120000);

await signInWithGitHub({ page, github });
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle sign-in failure from signInWithGitHub

The function signInWithGitHub returns a boolean indicating success or failure, but the return value is not checked. If the sign-in fails, the script will proceed, potentially causing unexpected errors later. Consider handling the result of the sign-in attempt.

Modify the code to check the return value:

- await signInWithGitHub({ page, github });
+ const signInSuccess = await signInWithGitHub({ page, github });
+ if (!signInSuccess) {
+   throw new Error('GitHub sign-in failed.');
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await signInWithGitHub({ page, github });
const signInSuccess = await signInWithGitHub({ page, github });
if (!signInSuccess) {
throw new Error('GitHub sign-in failed.');
}

await page.goto('/');
await page.getByRole('button', { name: 'Connect Wallet' }).click();
const page1Promise = page.waitForEvent('popup');
await page.getByRole('button', { name: 'GitHub' }).click();
const page1 = await page1Promise;

// GitHub will sometime ask for reauthorization
try {
await page1.waitForSelector('button:has-text("Authorize iam-login")', {
state: 'visible',
timeout: 10000,
});
console.log('Reauthorization screen detected');
await page1.getByRole('button', { name: 'Authorize iam-login' }).click();
} catch {
console.log('Reauthorization screen not detected. Continuing...');
}

// Web3Auth will sometimes ask for 2FA setup
try {
await page1.waitForSelector('text=Skip for now', {
state: 'visible',
timeout: 10000,
});
console.log('2FA setup screen detected');
await page1.getByRole('button', { name: 'Skip for now' }).click();
} catch {
console.log('2FA setup screen not detected. Continuing...');
}

await page1.waitForEvent('close', { timeout: 60000 });
await page.waitForSelector('button:has-text("Disconnect")', {
state: 'visible',
timeout: 20000,
});

await page.context().storageState({ path: authFile });
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Add playwright/.auth to .gitignore to prevent committing sensitive authentication data

Add the following line to .gitignore:

playwright/.auth

This will prevent accidentally committing authentication state files that may contain sensitive information like session tokens.

🔗 Analysis chain

Ensure authentication state file is excluded from version control

The authentication state is saved to ../playwright/.auth/user.json, which may contain sensitive information like session tokens. To prevent accidental commits of sensitive data, ensure this path is listed in .gitignore.

Run the following script to verify that playwright/.auth is included in .gitignore:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if 'playwright/.auth' is listed in .gitignore

if ! grep -q '^playwright/.auth' .gitignore; then
  echo "'playwright/.auth' is not in .gitignore. Please add it to prevent committing sensitive data."
else
  echo "'playwright/.auth' is correctly ignored."
fi

Length of output: 241

});
21 changes: 21 additions & 0 deletions e2e/bank.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { test, expect, Page } from '@playwright/test';
import { signInWithGitHub } from '@/e2e/utils/auth';

test.describe.serial('Bank', () => {
test('send mfx', async ({ page }) => {
await page.goto('/bank');
await expect(page.getByText('mfx', { exact: true })).toBeVisible();
await page.getByLabel('send-mfx').click();
await page.getByPlaceholder('0.00').click();
await page.getByPlaceholder('0.00').fill('1');
await page.getByPlaceholder('Enter address').click();
await page
.getByPlaceholder('Enter address')
.fill('manifest1hj5fveer5cjtn4wd6wstzugjfdxzl0xp8ws9ct');
Comment on lines +13 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Move test data to a separate constants file

Hardcoded values like addresses should be moved to a separate test constants file for better maintainability.

Create a new file e2e/constants.ts:

export const TEST_CONSTANTS = {
  RECIPIENT_ADDRESS: 'manifest1hj5fveer5cjtn4wd6wstzugjfdxzl0xp8ws9ct',
  TEST_AMOUNT: '1',
  TEST_MEMO: 'E2E bank test'
};

Then update the test to use these constants.

await page.getByPlaceholder('Memo').click();
await page.getByPlaceholder('Memo').fill('E2E bank test');
await page.getByLabel('send-btn').click();
await page.getByRole('button', { name: 'Approve' }).click();
await page.waitForSelector('text=Transaction Successful');
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add timeout and error handling for transaction confirmation

The waitForSelector call should include a reasonable timeout and error handling for failed transactions.

-    await page.waitForSelector('text=Transaction Successful');
+    try {
+      await page.waitForSelector('text=Transaction Successful', { timeout: 30000 });
+    } catch (error) {
+      throw new Error('Transaction did not complete within 30 seconds');
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await page.waitForSelector('text=Transaction Successful');
try {
await page.waitForSelector('text=Transaction Successful', { timeout: 30000 });
} catch (error) {
throw new Error('Transaction did not complete within 30 seconds');
}

});
Comment on lines +5 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve test robustness and organization

The test needs several improvements:

  1. Add error handling for transaction steps
  2. Validate transaction details before approval
  3. Break down the long test into smaller, focused test cases
  4. Add proper assertions for the transaction state

Here's a suggested refactor:

import { test, expect } from '@playwright/test';
import { TEST_CONSTANTS } from './constants';

test.describe('Bank Transactions', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto('/bank');
  });

  test('should display MFX balance', async ({ page }) => {
    await expect(page.getByTestId('mfx-balance')).toBeVisible();
  });

  test('should validate transaction form', async ({ page }) => {
    await page.getByLabel('send-mfx').click();
    
    // Test form validation
    await page.getByLabel('send-btn').click();
    await expect(page.getByText('Amount is required')).toBeVisible();
    
    // Fill invalid amount
    await page.getByPlaceholder('0.00').fill('-1');
    await expect(page.getByText('Amount must be positive')).toBeVisible();
  });

  test('should complete MFX transaction', async ({ page }) => {
    await page.getByLabel('send-mfx').click();
    
    // Fill transaction details
    await page.getByPlaceholder('0.00').fill(TEST_CONSTANTS.TEST_AMOUNT);
    await page.getByPlaceholder('Enter address').fill(TEST_CONSTANTS.RECIPIENT_ADDRESS);
    await page.getByPlaceholder('Memo').fill(TEST_CONSTANTS.TEST_MEMO);
    
    // Verify transaction details in confirmation dialog
    await page.getByLabel('send-btn').click();
    await expect(page.getByText(`Amount: ${TEST_CONSTANTS.TEST_AMOUNT} MFX`)).toBeVisible();
    await expect(page.getByText(`To: ${TEST_CONSTANTS.RECIPIENT_ADDRESS}`)).toBeVisible();
    
    // Complete transaction
    await page.getByRole('button', { name: 'Approve' }).click();
    
    // Verify success and balance update
    await expect(page.getByText('Transaction Successful')).toBeVisible();
    // Add assertion for updated balance
  });
});

});
12 changes: 12 additions & 0 deletions e2e/landing.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { test, expect } from '@playwright/test';

test('has title', async ({ page }) => {
await page.goto('/');
await expect(page).toHaveTitle(/Alberto - Blockchain App/);
});

test('learn how it works', async ({ page }) => {
await page.goto('/');
const button = page.locator('text=Learn how it works');
await expect(button).toBeVisible();
Comment on lines +10 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use data-testid for more reliable element selection

Using text content for element selection can be fragile as it may change with UI updates or internationalization. Consider using data-testid attributes.

-  const button = page.locator('text=Learn how it works');
+  const button = page.getByTestId('learn-more-button');

Also, consider adding a test for the button's click interaction and its expected behavior.

Committable suggestion skipped: line range outside the PR's diff.

});
31 changes: 31 additions & 0 deletions e2e/utils/auth.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { expect, Page } from '@playwright/test';

// Taken from https://github.com/Web3Auth/web3auth-e2e-tests/
export async function signInWithGitHub({
page,
github,
}: {
page: Page;
github: {
email: string;
password: string;
};
}): Promise<boolean> {
try {
await page.goto('https://github.com/login');
await page.waitForSelector('text=Sign in');
await page.isVisible('text=Sign in');

await page.fill('input[autocomplete="username"]', github.email);

await page.fill('input[autocomplete="current-password"]', github.password);

await page.click('input[value="Sign in"]');

await page.waitForSelector('text=Create repository');
expect(page.isVisible('text=Create repository')).toBe(true);
Comment on lines +25 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use a more reliable check for successful login

Waiting for 'text=Create repository' assumes the user's landing page contains this text, which may not always be true (e.g., for new accounts or if GitHub changes their UI). This could lead to false negatives in login verification.

Consider verifying the existence of the user's avatar or a consistent element in the navbar:

- await page.waitForSelector('text=Create repository');
- expect(page.isVisible('text=Create repository')).toBe(true);
+ await page.waitForSelector('summary[aria-label="View profile and more"]', {
+   state: 'visible',
+ });
+ expect(await page.isVisible('summary[aria-label="View profile and more"]')).toBe(true);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await page.waitForSelector('text=Create repository');
expect(page.isVisible('text=Create repository')).toBe(true);
await page.waitForSelector('summary[aria-label="View profile and more"]', {
state: 'visible',
});
expect(await page.isVisible('summary[aria-label="View profile and more"]')).toBe(true);

return true;
} catch {
return false;
}
}
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"format": "prettier --write .",
"remove": "rm -rf node_modules/ bun.lockb $HOME/.bun/install/cache/",
"update-deps": "bunx npm-check-updates --root --format group -i",
"test:coverage": "bun test --coverage",
"test:playwright": "bunx playwright test",
"test:coverage": "bun test components --coverage",
"test:coverage:lcov": "bun run test:coverage --coverage-reporter=lcov --coverage-dir ./coverage",
"coverage:upload": "codecov"
},
Expand Down Expand Up @@ -75,6 +76,7 @@
},
"devDependencies": {
"@happy-dom/global-registrator": "^15.7.3",
"@playwright/test": "^1.49.0",
"@tailwindcss/aspect-ratio": "^0.4.2",
"@tailwindcss/forms": "^0.5.3",
"@tailwindcss/typography": "^0.5.8",
Expand All @@ -84,13 +86,15 @@
"@types/bad-words": "^3.0.3",
"@types/crypto-js": "^4.2.2",
"@types/identicon.js": "^2.3.4",
"@types/node": "^22.9.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Verify @types/node version compatibility.

The @types/node version (^22.9.1) seems unusually high and might be incorrect, as Node.js itself is currently at version 20.x. This could lead to type definition incompatibilities.

-    "@types/node": "^22.9.1",
+    "@types/node": "^20.11.0",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"@types/node": "^22.9.1",
"@types/node": "^20.11.0",

"@types/react": "18.3.5",
"@types/react-dom": "18.3.0",
"@types/react-scroll": "^1.8.10",
"@types/three": "^0.169.0",
"bun-types": "^1.1.29",
"codecov": "^3.8.3",
"eslint": "8.56.0",
"dotenv": "^16.4.5",
"eslint": "8.57.1",
"eslint-config-next": "13.0.5",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-prettier": "^5.2.1",
Expand Down
86 changes: 86 additions & 0 deletions playwright.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { defineConfig, devices } from '@playwright/test';

/**
* Read environment variables from file.
* https://github.com/motdotla/dotenv
*/
import dotenv from 'dotenv';
import path from 'path';
dotenv.config({ path: path.resolve(__dirname, '.env.playwright') });

/**
* See https://playwright.dev/docs/test-configuration.
*/
export default defineConfig({
testDir: './e2e',
/* Run tests in files in parallel */
fullyParallel: true,
/* Fail the build on CI if you accidentally left test.only in the source code. */
forbidOnly: !!process.env.CI,
/* Retry on CI only */
retries: process.env.CI ? 2 : 0,
/* Opt out of parallel tests on CI. */
workers: process.env.CI ? 1 : undefined,
/* Reporter to use. See https://playwright.dev/docs/test-reporters */
reporter: 'html',
/* 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: 'http://127.0.0.1:3000',

/* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
trace: 'on-first-retry',
},

/* Configure projects for major browsers */
projects: [
{ name: 'setup', testMatch: /.*\.setup\.ts/ },

{
name: 'chromium',
use: { ...devices['Desktop Chrome'], storageState: 'playwright/.auth/user.json' },
dependencies: ['setup'],
},

// {
// name: 'firefox',
// use: { ...devices['Desktop Firefox'] },
// },

// {
// name: 'webkit',
// use: { ...devices['Desktop Safari'] },
// },

/* Test against mobile viewports. */
// {
// name: 'Mobile Chrome',
// use: { ...devices['Pixel 5'] },
// },
// {
// name: 'Mobile Chrome (landscape)',
// use: { ...devices['Pixel 5 landscape'] },
// },
// {
// name: 'Mobile Safari',
// use: { ...devices['iPhone 12'] },
// },

/* Test against branded browsers. */
// {
// name: 'Microsoft Edge',
// use: { ...devices['Desktop Edge'], channel: 'msedge' },
// },
// {
// name: 'Google Chrome',
// use: { ...devices['Desktop Chrome'], channel: 'chrome' },
// },
],

/* Run your local dev server before starting the tests */
webServer: {
command: 'bun run start',
url: 'http://127.0.0.1:3000',
reuseExistingServer: !process.env.CI,
},
Comment on lines +80 to +85
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add health check URL for reliable server startup.

The webServer configuration should include a health check URL to ensure the server is fully ready before tests begin.

  webServer: {
    command: 'bun run start',
    url: 'http://127.0.0.1:3000',
+   reuseExistingServer: !process.env.CI,
+   stdout: 'pipe',
+   stderr: 'pipe',
+   timeout: 120000,
  },

Committable suggestion skipped: line range outside the PR's diff.

});
Loading