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] Use playwright electron module for e2e testing #888

Merged
merged 29 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f34a2ae
Use playwright electron module for e2e testing
webfiltered Feb 11, 2025
a5cd44b
nit - Rename
webfiltered Feb 11, 2025
be7f239
Backup local user config on global setup
webfiltered Feb 12, 2025
97a812c
Set more realistic timeouts
webfiltered Feb 12, 2025
893cbdc
Refactor TestApp to base class
webfiltered Feb 12, 2025
70f9f82
Split app window tests to separate file
webfiltered Feb 12, 2025
d61a475
Add self-cleaning test app class to run installation tests
webfiltered Feb 12, 2025
36b1973
Add basic testing README
webfiltered Feb 12, 2025
fbe148b
Update terminology - now using e2e testing
webfiltered Feb 12, 2025
6e23b24
Include playwright reports in GH action artifacts
webfiltered Feb 12, 2025
4366366
Add Playwright reporting
webfiltered Feb 12, 2025
c8eab76
Use default expect timeout
webfiltered Feb 12, 2025
e0f7b3e
Capture trace, screenshots, and video in CI
webfiltered Feb 12, 2025
3afa0b7
Fix workflow - default Playwright output dir
webfiltered Feb 12, 2025
e573021
Add log upload
webfiltered Feb 12, 2025
6896f67
Add screenshot on app start
webfiltered Feb 12, 2025
724a333
nit - Add step names
webfiltered Feb 12, 2025
a41d8d0
Copy log file location from runner shell to GH env
webfiltered Feb 12, 2025
15000a4
Fix GH action syntax
webfiltered Feb 12, 2025
b31e31e
Attach desktop app logs before resetting appdata
webfiltered Feb 12, 2025
2fe6e1c
Fix throw when log missing
webfiltered Feb 12, 2025
9440da9
Add app startup timeout
webfiltered Feb 12, 2025
e18576f
Use debug log level in e2e tests
webfiltered Feb 12, 2025
7cc1afe
Add test screenshot
webfiltered Feb 12, 2025
f5e8719
Allow env vars to be passed into tested app
webfiltered Feb 12, 2025
f13f409
Prefer Playwright attach to GH action upload
webfiltered Feb 12, 2025
95f382a
nit
webfiltered Feb 12, 2025
6271ff4
Remove debug code
webfiltered Feb 12, 2025
055b899
nit
webfiltered Feb 12, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/integration_test_windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jobs:
runs-on: windows-latest
env:
SKIP_HARDWARE_VALIDATION: 'true'
LOG_LEVEL: 'debug'
steps:
- name: Github checkout
uses: actions/checkout@v4
Expand All @@ -43,6 +44,14 @@ jobs:
run: npm run test:e2e
- name: Upload screenshots
uses: actions/upload-artifact@v4
if: ${{ !cancelled() }}
with:
name: screenshot
path: screenshot*.png
- name: Upload Test Results
uses: actions/upload-artifact@v4
if: ${{ !cancelled() }}
with:
name: test-results
path: test-results/
retention-days: 30
15 changes: 15 additions & 0 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,19 @@ export default defineConfig({
testDir: './tests/integration',
/* Run local instance before starting the tests */
globalSetup: './playwright.setup',
// Per-test timeout - 60 sec
timeout: 60_000,
// Entire test suite timeout - 1 hour
globalTimeout: 60 * 60 * 1000,
// This is a desktop app; sharding is required to run tests in parallel.
workers: 1,
// GitHub reporter in CI, dot reporter for local development.
reporter: process.env.CI ? 'github' : 'dot',
// Capture trace, screenshots, and video on first retry in CI.
retries: process.env.CI ? 1 : 0,
use: {
screenshot: 'only-on-failure',
trace: 'on-first-retry',
video: 'on-first-retry',
},
});
27 changes: 14 additions & 13 deletions playwright.setup.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
import { spawn } from 'node:child_process';
import { rename } from 'node:fs/promises';

import { FilePermission, addRandomSuffix, getComfyUIAppDataPath, pathExists } from './tests/shared/utils';

async function globalSetup() {
console.log('Playwright globalSetup called');
console.log('+ Playwright globalSetup called');
if (process.env.CI) return;

return new Promise<void>((resolve, reject) => {
const electron = spawn('node', ['./scripts/launchCI.js']);
const appDataPath = getComfyUIAppDataPath();
await backupByRenaming(appDataPath);
}

electron.on('close', () => {
reject(new Error('process failed to start'));
});
async function backupByRenaming(appDataPath: string) {
if (!(await pathExists(appDataPath, FilePermission.Writable))) return;

electron.stdout.on('data', (data: string | Buffer) => {
if (data.includes('App ready')) {
resolve();
}
});
});
const newPath = addRandomSuffix(appDataPath);
console.warn(`AppData exists! Moving ${appDataPath} to ${newPath}. Remove manually if you do not require it.`);
await rename(appDataPath, newPath);
return newPath;
}

export default globalSetup;
34 changes: 34 additions & 0 deletions tests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Testing

## Unit Tests

Unit tests are run with vitest. Tests are run in parallel.

### Running

```bash
yarn run test:unit
```

## End-to-End Tests

End-to-end tests are run with Playwright. Tests are run sequentially.

Tests are intended to be run on virtualised, disposable systems, such as CI runners.

> [!CAUTION]
> End-to-end tests erase settings and other app data. They will delete ComfyUI directories without warning.

### Running

```bash
yarn run test:e2e
```

> [!NOTE]
> As a precaution, if the app data directory already exists, it will have a random suffix appended to its name.

App data directories:

- `%APPDATA%\ComfyUI` (Windows)
- `Application Support/ComfyUI` (Mac)
84 changes: 34 additions & 50 deletions tests/integration/appLifecycle.spec.ts
Original file line number Diff line number Diff line change
@@ -1,68 +1,52 @@
import { type Locator, chromium, expect, test } from '@playwright/test';
import { type Locator, expect } from '@playwright/test';

test('has title', async () => {
const browser = await chromium.connectOverCDP('http://127.0.0.1:9000');
import { test } from './autoCleaningTestApp';

expect(browser.isConnected()).toBeTruthy();
expect(browser.contexts().length).toBeGreaterThan(0);
const APP_START_TIMEOUT = process.env.CI ? 30_000 : 5000;

const context = browser.contexts()[0];
const pages = context.pages();
test.describe('App Lifecycle', () => {
test('does all app startup things from previous test', async ({ autoCleaningApp }) => {
const window = await autoCleaningApp.firstWindow();
await window.screenshot({ path: 'screenshot-app-start.png' });

expect(pages).toHaveLength(1);
const page = pages[0];
const getStartedButton = window.getByText('Get Started');

// Expect a title "to contain" a substring.
await expect(page).toHaveTitle(/ComfyUI/);
await expect(getStartedButton).toBeVisible({ timeout: APP_START_TIMEOUT });
await expect(getStartedButton).toBeEnabled();

const getStartedButton = page.getByText('Get Started');
await window.screenshot({ path: 'screenshot-load.png' });

await expect(getStartedButton).toBeVisible();
await expect(getStartedButton).toBeEnabled();
await getStartedButton.click();

await page.screenshot({ path: 'screenshot-load.png' });
// Select GPU screen
await expect(window.getByText('Select GPU')).toBeVisible();

await getStartedButton.click();
const nextButton = window.getByRole('button', { name: 'Next' });
const cpuToggle = window.locator('#cpu-mode');

// Select GPU screen
await expect(page.getByText('Select GPU')).toBeVisible();
await expect(cpuToggle).toBeVisible();
await cpuToggle.click();

const nextButton = page.getByRole('button', { name: 'Next' });
const cpuToggle = page.locator('#cpu-mode');
await clickEnabledButton(nextButton);

await expect(cpuToggle).toBeVisible();
await cpuToggle.click();
await expect(window.getByText('Choose Installation Location')).toBeVisible();
await window.screenshot({ path: 'screenshot-get-started.png' });

await clickEnabledButton(nextButton);
await clickEnabledButton(nextButton);

await expect(page.getByText('Choose Installation Location')).toBeVisible();
await page.screenshot({ path: 'screenshot-get-started.png' });
await expect(window.getByText('Migrate from Existing Installation')).toBeVisible();
await window.screenshot({ path: 'screenshot-migrate.png' });

await clickEnabledButton(nextButton);
await clickEnabledButton(nextButton);

await expect(page.getByText('Migrate from Existing Installation')).toBeVisible();
await page.screenshot({ path: 'screenshot-migrate.png' });
await expect(window.getByText('Desktop App Settings')).toBeVisible();
await window.screenshot({ path: 'screenshot-install.png' });

await clickEnabledButton(nextButton);

await expect(page.getByText('Desktop App Settings')).toBeVisible();
await page.screenshot({ path: 'screenshot-install.png' });

/** Ensure a button is enabled, then click it. */
async function clickEnabledButton(button: Locator) {
await expect(button).toBeVisible();
await expect(button).toBeEnabled();
await button.click();
}
});

test('app quits when window is closed', async () => {
const browser = await chromium.connectOverCDP('http://127.0.0.1:9000');
const context = browser.contexts()[0];
const page = context.pages()[0];

await page.close();
await new Promise((resolve) => setTimeout(resolve, 3000));

expect(browser.isConnected()).toBeFalsy();
/** Ensure a button is enabled, then click it. */
async function clickEnabledButton(button: Locator) {
await expect(button).toBeVisible();
await expect(button).toBeEnabled();
await button.click();
}
});
});
15 changes: 15 additions & 0 deletions tests/integration/appWindow.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { expect } from '@playwright/test';

import { test } from './testApp';

test('App window has title', async ({ app }) => {
const window = await app.firstWindow();
await expect(window).toHaveTitle('ComfyUI');
});

test('App quits when window is closed', async ({ app }) => {
const window = await app.firstWindow();

await window.close();
await app.app.waitForEvent('close');
});
41 changes: 41 additions & 0 deletions tests/integration/autoCleaningTestApp.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { type TestInfo, test as baseTest } from '@playwright/test';
import { pathExists } from 'tests/shared/utils';

import { TestApp } from './testApp';
import { TestEnvironment } from './testEnvironment';

async function attachIfExists(testInfo: TestInfo, path: string) {
if (await pathExists(path)) {
await testInfo.attach('main.log', { path });
}
}

export const test = baseTest.extend<{ autoCleaningApp: AutoCleaningTestApp }>({
autoCleaningApp: async ({}, use, testInfo) => {
// Launch Electron app.
await using app = await AutoCleaningTestApp.create();
await use(app);

// After test
const appEnv = app.testEnvironment;
await attachIfExists(testInfo, appEnv.mainLogPath);
await attachIfExists(testInfo, appEnv.comfyuiLogPath);
},
});

/**
* {@link TestApp} that cleans up AppData and the install directory when disposed.
*/
export class AutoCleaningTestApp extends TestApp implements AsyncDisposable {
readonly testEnvironment: TestEnvironment = new TestEnvironment();

static async create() {
const app = await TestApp.launchElectron();
return new AutoCleaningTestApp(app);
}

async [Symbol.asyncDispose](): Promise<void> {
await super[Symbol.asyncDispose]();
await this.testEnvironment[Symbol.asyncDispose]();
}
}
14 changes: 14 additions & 0 deletions tests/integration/tempDirectory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import path from 'node:path';
import { addRandomSuffix, pathExists } from 'tests/shared/utils';

export class TempDirectory implements AsyncDisposable {
readonly installLocation: string = path.join(tmpdir(), addRandomSuffix('ComfyUI'));

async [Symbol.asyncDispose](): Promise<void> {
if (await pathExists(this.installLocation)) {
await rm(this.installLocation, { recursive: true, force: true });
}
}
}
61 changes: 61 additions & 0 deletions tests/integration/testApp.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { type ElectronApplication, test as baseTest } from '@playwright/test';
import electronPath from 'electron';
import { _electron as electron } from 'playwright';

// eslint-disable-next-line @typescript-eslint/no-base-to-string
const executablePath = String(electronPath);

const isCI = !!process.env.CI;

// Extend the base test
export const test = baseTest.extend<{ app: TestApp }>({
app: async ({}, use) => {
// Launch Electron app.
await using app = await TestApp.create();
await use(app);
},
});

// Local testing QoL
async function localTestQoL(app: ElectronApplication) {
if (isCI) return;

// Get the first window that the app opens, wait if necessary.
const window = await app.firstWindow();
// Direct Electron console to Node terminal.
window.on('console', console.log);
}

/**
* Base class for desktop e2e tests.
*/
export class TestApp implements AsyncDisposable {
protected constructor(readonly app: ElectronApplication) {}

/** Async static factory */
static async create() {
const app = await TestApp.launchElectron();
return new TestApp(app);
}

/** Get the first window that the app opens. Wait if necessary. */
async firstWindow() {
return await this.app.firstWindow();
}

/** Executes the Electron app. If not in CI, logs browser console via `console.log()`. */
protected static async launchElectron() {
const app = await electron.launch({
args: ['.'],
executablePath,
cwd: '.',
});
await localTestQoL(app);
return app;
}

/** Dispose: close the app. */
async [Symbol.asyncDispose](): Promise<void> {
await this.app[Symbol.asyncDispose]();
}
}
35 changes: 35 additions & 0 deletions tests/integration/testEnvironment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { rm } from 'node:fs/promises';
import path from 'node:path';
import { getComfyUIAppDataPath } from 'tests/shared/utils';

import { TempDirectory } from './tempDirectory';

export class TestEnvironment implements AsyncDisposable {
readonly appDataDir: string = getComfyUIAppDataPath();
readonly installLocation: TempDirectory = new TempDirectory();

readonly mainLogPath: string;
readonly comfyuiLogPath: string;

constructor() {
this.mainLogPath = path.join(this.appDataDir, 'logs', 'main.log');
this.comfyuiLogPath = path.join(this.appDataDir, 'logs', 'comfyui.log');
}

async deleteEverything() {
await this.deleteAppData();
await this.deleteInstallLocation();
}

async deleteAppData() {
await rm(this.appDataDir, { recursive: true, force: true });
}

async deleteInstallLocation() {
await this.installLocation[Symbol.asyncDispose]();
}

async [Symbol.asyncDispose](): Promise<void> {
await this.deleteEverything();
}
}
Loading
Loading