From f34a2aee6860379e88ce33be39ad02670226f741 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Tue, 11 Feb 2025 17:32:52 +1100 Subject: [PATCH 01/29] Use playwright electron module for e2e testing --- playwright.setup.ts | 20 +----- tests/integration/appLifecycle.spec.ts | 90 ++++++++++++-------------- tests/integration/testApp.ts | 48 ++++++++++++++ 3 files changed, 93 insertions(+), 65 deletions(-) create mode 100644 tests/integration/testApp.ts diff --git a/playwright.setup.ts b/playwright.setup.ts index 400e238a..caac416d 100644 --- a/playwright.setup.ts +++ b/playwright.setup.ts @@ -1,21 +1,5 @@ -import { spawn } from 'node:child_process'; - -async function globalSetup() { - console.log('Playwright globalSetup called'); - - return new Promise((resolve, reject) => { - const electron = spawn('node', ['./scripts/launchCI.js']); - - electron.on('close', () => { - reject(new Error('process failed to start')); - }); - - electron.stdout.on('data', (data: string | Buffer) => { - if (data.includes('App ready')) { - resolve(); - } - }); - }); +function globalSetup() { + console.log('+ Playwright globalSetup called'); } export default globalSetup; diff --git a/tests/integration/appLifecycle.spec.ts b/tests/integration/appLifecycle.spec.ts index 95662670..2e77f236 100644 --- a/tests/integration/appLifecycle.spec.ts +++ b/tests/integration/appLifecycle.spec.ts @@ -1,68 +1,64 @@ -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 './testApp'; - expect(browser.isConnected()).toBeTruthy(); - expect(browser.contexts().length).toBeGreaterThan(0); +test.describe('App Lifecycle', () => { + test('has title', async ({ testApp }) => { + const window = await testApp.firstWindow(); + await expect(window).toHaveTitle('ComfyUI'); + }); - const context = browser.contexts()[0]; - const pages = context.pages(); + test('does all app startup things from previous test', async ({ testApp }) => { + const page = await testApp.firstWindow(); - expect(pages).toHaveLength(1); - const page = pages[0]; + // Expect a title "to contain" a substring. + await expect(page).toHaveTitle(/ComfyUI/); - // Expect a title "to contain" a substring. - await expect(page).toHaveTitle(/ComfyUI/); + const getStartedButton = page.getByText('Get Started'); - const getStartedButton = page.getByText('Get Started'); + await expect(getStartedButton).toBeVisible(); + await expect(getStartedButton).toBeEnabled(); - await expect(getStartedButton).toBeVisible(); - await expect(getStartedButton).toBeEnabled(); + await page.screenshot({ path: 'screenshot-load.png' }); - await page.screenshot({ path: 'screenshot-load.png' }); + await getStartedButton.click(); - await getStartedButton.click(); + // Select GPU screen + await expect(page.getByText('Select GPU')).toBeVisible(); - // Select GPU screen - await expect(page.getByText('Select GPU')).toBeVisible(); + const nextButton = page.getByRole('button', { name: 'Next' }); + const cpuToggle = page.locator('#cpu-mode'); - const nextButton = page.getByRole('button', { name: 'Next' }); - const cpuToggle = page.locator('#cpu-mode'); + await expect(cpuToggle).toBeVisible(); + await cpuToggle.click(); - await expect(cpuToggle).toBeVisible(); - await cpuToggle.click(); + await clickEnabledButton(nextButton); - await clickEnabledButton(nextButton); + await expect(page.getByText('Choose Installation Location')).toBeVisible(); + await page.screenshot({ path: 'screenshot-get-started.png' }); - await expect(page.getByText('Choose Installation Location')).toBeVisible(); - await page.screenshot({ path: 'screenshot-get-started.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(page.getByText('Migrate from Existing Installation')).toBeVisible(); - await page.screenshot({ path: 'screenshot-migrate.png' }); + await clickEnabledButton(nextButton); - await clickEnabledButton(nextButton); + await expect(page.getByText('Desktop App Settings')).toBeVisible(); + await page.screenshot({ path: 'screenshot-install.png' }); - 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(); + } + }); - /** 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)); + test('app quits when window is closed', async ({ testApp }) => { + const window = await testApp.firstWindow(); - expect(browser.isConnected()).toBeFalsy(); + await window.close(); + await testApp.app.waitForEvent('close'); + }); }); diff --git a/tests/integration/testApp.ts b/tests/integration/testApp.ts new file mode 100644 index 00000000..3c54b075 --- /dev/null +++ b/tests/integration/testApp.ts @@ -0,0 +1,48 @@ +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; + +export const test = baseTest.extend<{ testApp: TestApp }>({ + testApp: async ({}, use) => { + // Launch Electron app. + await using testApp = await TestApp.create(); + await use(testApp); + }, +}); + +export class TestApp implements AsyncDisposable { + private constructor(readonly app: ElectronApplication) {} + + static async create() { + const app = await electron.launch({ + args: ['.'], + executablePath, + cwd: '.', + env: {}, + }); + const testApp = new TestApp(app); + + // Local testing QoL + if (!isCI) { + // Get the first window that the app opens, wait if necessary. + const window = await testApp.firstWindow(); + // Direct Electron console to Node terminal. + window.on('console', console.log); + } + + return testApp; + } + + async firstWindow() { + return this.app.firstWindow(); + } + + async [Symbol.asyncDispose](): Promise { + await this.app[Symbol.asyncDispose](); + } +} From a5cd44b1eb3f6a5f0ae16ea843a296f212b87c83 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Tue, 11 Feb 2025 18:21:51 +1100 Subject: [PATCH 02/29] nit - Rename --- tests/integration/appLifecycle.spec.ts | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/integration/appLifecycle.spec.ts b/tests/integration/appLifecycle.spec.ts index 2e77f236..724bbf41 100644 --- a/tests/integration/appLifecycle.spec.ts +++ b/tests/integration/appLifecycle.spec.ts @@ -9,43 +9,43 @@ test.describe('App Lifecycle', () => { }); test('does all app startup things from previous test', async ({ testApp }) => { - const page = await testApp.firstWindow(); + const window = await testApp.firstWindow(); // Expect a title "to contain" a substring. - await expect(page).toHaveTitle(/ComfyUI/); + await expect(window).toHaveTitle(/ComfyUI/); - const getStartedButton = page.getByText('Get Started'); + const getStartedButton = window.getByText('Get Started'); await expect(getStartedButton).toBeVisible(); await expect(getStartedButton).toBeEnabled(); - await page.screenshot({ path: 'screenshot-load.png' }); + await window.screenshot({ path: 'screenshot-load.png' }); await getStartedButton.click(); // Select GPU screen - await expect(page.getByText('Select GPU')).toBeVisible(); + await expect(window.getByText('Select GPU')).toBeVisible(); - const nextButton = page.getByRole('button', { name: 'Next' }); - const cpuToggle = page.locator('#cpu-mode'); + const nextButton = window.getByRole('button', { name: 'Next' }); + const cpuToggle = window.locator('#cpu-mode'); await expect(cpuToggle).toBeVisible(); await cpuToggle.click(); await clickEnabledButton(nextButton); - await expect(page.getByText('Choose Installation Location')).toBeVisible(); - await page.screenshot({ path: 'screenshot-get-started.png' }); + await expect(window.getByText('Choose Installation Location')).toBeVisible(); + await window.screenshot({ path: 'screenshot-get-started.png' }); await clickEnabledButton(nextButton); - await expect(page.getByText('Migrate from Existing Installation')).toBeVisible(); - await page.screenshot({ path: 'screenshot-migrate.png' }); + await expect(window.getByText('Migrate from Existing Installation')).toBeVisible(); + await window.screenshot({ path: 'screenshot-migrate.png' }); await clickEnabledButton(nextButton); - await expect(page.getByText('Desktop App Settings')).toBeVisible(); - await page.screenshot({ path: 'screenshot-install.png' }); + await expect(window.getByText('Desktop App Settings')).toBeVisible(); + await window.screenshot({ path: 'screenshot-install.png' }); /** Ensure a button is enabled, then click it. */ async function clickEnabledButton(button: Locator) { From be7f2394e842cd0ed044de7a7430b0ebab3c65c5 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 16:15:15 +1100 Subject: [PATCH 03/29] Backup local user config on global setup --- playwright.setup.ts | 19 ++++++++++++++++++- tests/shared/utils.ts | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 tests/shared/utils.ts diff --git a/playwright.setup.ts b/playwright.setup.ts index caac416d..b4848b7b 100644 --- a/playwright.setup.ts +++ b/playwright.setup.ts @@ -1,5 +1,22 @@ -function globalSetup() { +import { rename } from 'node:fs/promises'; + +import { FilePermission, addRandomSuffix, getComfyUIAppDataPath, pathExists } from './tests/shared/utils'; + +async function globalSetup() { console.log('+ Playwright globalSetup called'); + if (process.env.CI) return; + + const appDataPath = getComfyUIAppDataPath(); + await backupByRenaming(appDataPath); +} + +async function backupByRenaming(appDataPath: string) { + if (!(await pathExists(appDataPath, FilePermission.Writable))) return; + + 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; diff --git a/tests/shared/utils.ts b/tests/shared/utils.ts new file mode 100644 index 00000000..e2020e05 --- /dev/null +++ b/tests/shared/utils.ts @@ -0,0 +1,38 @@ +import { randomUUID } from 'node:crypto'; +import { access, constants } from 'node:fs/promises'; +import { homedir } from 'node:os'; +import path from 'node:path'; + +// Dumping ground for basic utilities that can be shared by e2e and unit tests + +export enum FilePermission { + Exists = constants.F_OK, + Readable = constants.R_OK, + Writable = constants.W_OK, + Executable = constants.X_OK, +} + +export async function pathExists(path: string, permission: FilePermission = FilePermission.Exists) { + try { + await access(path, permission); + return true; + } catch { + return false; + } +} + +export function getComfyUIAppDataPath() { + switch (process.platform) { + case 'win32': + if (!process.env.APPDATA) throw new Error('APPDATA environment variable is not set.'); + return path.join(process.env.APPDATA, 'ComfyUI'); + case 'darwin': + return path.join(homedir(), 'Library', 'Application Support', 'ComfyUI'); + default: + return path.join(homedir(), '.config', 'ComfyUI'); + } +} + +export function addRandomSuffix(str: string) { + return `${str}-${randomUUID().substring(0, 8)}`; +} From 97a812c0c904918b30fc8ea9418533dd061e3f7e Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 16:32:30 +1100 Subject: [PATCH 04/29] Set more realistic timeouts --- playwright.config.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/playwright.config.ts b/playwright.config.ts index c07c405f..77b4df95 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -4,4 +4,13 @@ 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, + expect: { + timeout: 10_000, + }, + // This is a desktop app; sharding is required to run tests in parallel. + workers: 1, }); From 893cbdc66ac7e0d464bdec3b1c663b2c00fbf45a Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 16:33:44 +1100 Subject: [PATCH 05/29] Refactor TestApp to base class --- tests/integration/testApp.ts | 54 +++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/tests/integration/testApp.ts b/tests/integration/testApp.ts index 3c54b075..ba2cc978 100644 --- a/tests/integration/testApp.ts +++ b/tests/integration/testApp.ts @@ -7,41 +7,55 @@ const executablePath = String(electronPath); const isCI = !!process.env.CI; -export const test = baseTest.extend<{ testApp: TestApp }>({ - testApp: async ({}, use) => { +// Extend the base test +export const test = baseTest.extend<{ app: TestApp }>({ + app: async ({}, use) => { // Launch Electron app. - await using testApp = await TestApp.create(); - await use(testApp); + 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 { - private constructor(readonly app: ElectronApplication) {} + 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: '.', env: {}, }); - const testApp = new TestApp(app); - - // Local testing QoL - if (!isCI) { - // Get the first window that the app opens, wait if necessary. - const window = await testApp.firstWindow(); - // Direct Electron console to Node terminal. - window.on('console', console.log); - } - - return testApp; - } - - async firstWindow() { - return this.app.firstWindow(); + await localTestQoL(app); + return app; } + /** Dispose: close the app. */ async [Symbol.asyncDispose](): Promise { await this.app[Symbol.asyncDispose](); } From 70f9f820e784977f3d5e6cf135b26426b51d1816 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 16:36:02 +1100 Subject: [PATCH 06/29] Split app window tests to separate file --- tests/integration/appLifecycle.spec.ts | 15 --------------- tests/integration/appWindow.spec.ts | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 15 deletions(-) create mode 100644 tests/integration/appWindow.spec.ts diff --git a/tests/integration/appLifecycle.spec.ts b/tests/integration/appLifecycle.spec.ts index 724bbf41..8822ac3e 100644 --- a/tests/integration/appLifecycle.spec.ts +++ b/tests/integration/appLifecycle.spec.ts @@ -3,17 +3,9 @@ import { type Locator, expect } from '@playwright/test'; import { test } from './testApp'; test.describe('App Lifecycle', () => { - test('has title', async ({ testApp }) => { - const window = await testApp.firstWindow(); - await expect(window).toHaveTitle('ComfyUI'); - }); - test('does all app startup things from previous test', async ({ testApp }) => { const window = await testApp.firstWindow(); - // Expect a title "to contain" a substring. - await expect(window).toHaveTitle(/ComfyUI/); - const getStartedButton = window.getByText('Get Started'); await expect(getStartedButton).toBeVisible(); @@ -54,11 +46,4 @@ test.describe('App Lifecycle', () => { await button.click(); } }); - - test('app quits when window is closed', async ({ testApp }) => { - const window = await testApp.firstWindow(); - - await window.close(); - await testApp.app.waitForEvent('close'); - }); }); diff --git a/tests/integration/appWindow.spec.ts b/tests/integration/appWindow.spec.ts new file mode 100644 index 00000000..ce463b87 --- /dev/null +++ b/tests/integration/appWindow.spec.ts @@ -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'); +}); From d61a475647c5135e1d3d87014c477c56ffdb364b Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 16:36:34 +1100 Subject: [PATCH 07/29] Add self-cleaning test app class to run installation tests --- tests/integration/appLifecycle.spec.ts | 6 ++--- tests/integration/autoCleaningTestApp.ts | 29 ++++++++++++++++++++++++ tests/integration/tempDirectory.ts | 14 ++++++++++++ tests/integration/testEnvironment.ts | 26 +++++++++++++++++++++ 4 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 tests/integration/autoCleaningTestApp.ts create mode 100644 tests/integration/tempDirectory.ts create mode 100644 tests/integration/testEnvironment.ts diff --git a/tests/integration/appLifecycle.spec.ts b/tests/integration/appLifecycle.spec.ts index 8822ac3e..9c2f1b6a 100644 --- a/tests/integration/appLifecycle.spec.ts +++ b/tests/integration/appLifecycle.spec.ts @@ -1,10 +1,10 @@ import { type Locator, expect } from '@playwright/test'; -import { test } from './testApp'; +import { test } from './autoCleaningTestApp'; test.describe('App Lifecycle', () => { - test('does all app startup things from previous test', async ({ testApp }) => { - const window = await testApp.firstWindow(); + test('does all app startup things from previous test', async ({ autoCleaningApp }) => { + const window = await autoCleaningApp.firstWindow(); const getStartedButton = window.getByText('Get Started'); diff --git a/tests/integration/autoCleaningTestApp.ts b/tests/integration/autoCleaningTestApp.ts new file mode 100644 index 00000000..002c1793 --- /dev/null +++ b/tests/integration/autoCleaningTestApp.ts @@ -0,0 +1,29 @@ +import { test as testBase } from '@playwright/test'; + +import { TestApp } from './testApp'; +import { TestEnvironment } from './testEnvironment'; + +export const test = testBase.extend<{ autoCleaningApp: AutoCleaningTestApp }>({ + autoCleaningApp: async ({}, use) => { + // Launch Electron app. + await using app = await AutoCleaningTestApp.create(); + await use(app); + }, +}); + +/** + * {@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 { + await super[Symbol.asyncDispose](); + await this.testEnvironment[Symbol.asyncDispose](); + } +} diff --git a/tests/integration/tempDirectory.ts b/tests/integration/tempDirectory.ts new file mode 100644 index 00000000..12ddc774 --- /dev/null +++ b/tests/integration/tempDirectory.ts @@ -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 { + if (await pathExists(this.installLocation)) { + await rm(this.installLocation, { recursive: true, force: true }); + } + } +} diff --git a/tests/integration/testEnvironment.ts b/tests/integration/testEnvironment.ts new file mode 100644 index 00000000..6eb8db64 --- /dev/null +++ b/tests/integration/testEnvironment.ts @@ -0,0 +1,26 @@ +import { rm } from 'node:fs/promises'; +import { getComfyUIAppDataPath } from 'tests/shared/utils'; + +import { TempDirectory } from './tempDirectory'; + +export class TestEnvironment implements AsyncDisposable { + readonly appDataDir: string = getComfyUIAppDataPath(); + readonly installLocation: TempDirectory = new TempDirectory(); + + 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 { + await this.deleteEverything(); + } +} From 36b19730befe0ebf50903111ec71b7e5ba377acc Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 16:36:42 +1100 Subject: [PATCH 08/29] Add basic testing README --- tests/README.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 tests/README.md diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 00000000..18097915 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,34 @@ +# Testing + +## Unit Tests + +Unit tests are run with vitest. Tests are run in parallel. + +### Running + +```bash +yarn run test:unit +``` + +## Integration Tests + +Integration tests are run with Playwright. Tests are run sequentially. + +> [!CAUTION] +> Integration tests erase settings and other app data. They will delete ComfyUI directories without warning. + +These tests are designed to be run in CI or a virtual machine. + +### Running + +```bash +yarn run test:integration +``` + +> [!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) From fbe148b16e2876410029fb455d66ded31ce5fe51 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 17:06:59 +1100 Subject: [PATCH 09/29] Update terminology - now using e2e testing --- tests/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/README.md b/tests/README.md index 18097915..e0ad8e94 100644 --- a/tests/README.md +++ b/tests/README.md @@ -10,19 +10,19 @@ Unit tests are run with vitest. Tests are run in parallel. yarn run test:unit ``` -## Integration Tests +## End-to-End Tests -Integration tests are run with Playwright. Tests are run sequentially. +End-to-end tests are run with Playwright. Tests are run sequentially. > [!CAUTION] -> Integration tests erase settings and other app data. They will delete ComfyUI directories without warning. +> End-to-end tests erase settings and other app data. They will delete ComfyUI directories without warning. These tests are designed to be run in CI or a virtual machine. ### Running ```bash -yarn run test:integration +yarn run test:e2e ``` > [!NOTE] From 6e23b2422d989b2462b970821d87261116ea864a Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 17:14:48 +1100 Subject: [PATCH 10/29] Include playwright reports in GH action artifacts --- .github/workflows/integration_test_windows.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/integration_test_windows.yml b/.github/workflows/integration_test_windows.yml index 66e694ff..5aedec30 100644 --- a/.github/workflows/integration_test_windows.yml +++ b/.github/workflows/integration_test_windows.yml @@ -43,6 +43,13 @@ jobs: run: npm run test:e2e - name: Upload screenshots uses: actions/upload-artifact@v4 + if: ${{ !cancelled() }} with: name: screenshot path: screenshot*.png + - uses: actions/upload-artifact@v4 + if: ${{ !cancelled() }} + with: + name: playwright-report + path: playwright-report/ + retention-days: 30 From 4366366831b0415ebb4c2c6d4d2aefcef6b1f55a Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 18:34:48 +1100 Subject: [PATCH 11/29] Add Playwright reporting --- playwright.config.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/playwright.config.ts b/playwright.config.ts index 77b4df95..a7d7af00 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -13,4 +13,6 @@ export default defineConfig({ }, // 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', }); From c8eab760d8013577f949983d26bf5f317d0f5083 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 18:51:39 +1100 Subject: [PATCH 12/29] Use default expect timeout --- playwright.config.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/playwright.config.ts b/playwright.config.ts index a7d7af00..72345e7b 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -8,9 +8,6 @@ export default defineConfig({ timeout: 60_000, // Entire test suite timeout - 1 hour globalTimeout: 60 * 60 * 1000, - expect: { - timeout: 10_000, - }, // This is a desktop app; sharding is required to run tests in parallel. workers: 1, // GitHub reporter in CI, dot reporter for local development. From e0f7b3e6bf39d72b2271485d4c9fd9f3543bfec4 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 19:16:18 +1100 Subject: [PATCH 13/29] Capture trace, screenshots, and video in CI --- playwright.config.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/playwright.config.ts b/playwright.config.ts index 72345e7b..e8058430 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -12,4 +12,11 @@ export default defineConfig({ 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', + }, }); From 3afa0b7263f7f4349ee9309e1f52a9687f525010 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 19:45:28 +1100 Subject: [PATCH 14/29] Fix workflow - default Playwright output dir --- .github/workflows/integration_test_windows.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/integration_test_windows.yml b/.github/workflows/integration_test_windows.yml index 5aedec30..1ab1c627 100644 --- a/.github/workflows/integration_test_windows.yml +++ b/.github/workflows/integration_test_windows.yml @@ -50,6 +50,6 @@ jobs: - uses: actions/upload-artifact@v4 if: ${{ !cancelled() }} with: - name: playwright-report - path: playwright-report/ + name: test-results + path: test-results/ retention-days: 30 From e573021d54bd00a44dcd1d4971d155174f544a44 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 20:45:37 +1100 Subject: [PATCH 15/29] Add log upload --- .github/workflows/integration_test_windows.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/integration_test_windows.yml b/.github/workflows/integration_test_windows.yml index 1ab1c627..7666d3b3 100644 --- a/.github/workflows/integration_test_windows.yml +++ b/.github/workflows/integration_test_windows.yml @@ -53,3 +53,9 @@ jobs: name: test-results path: test-results/ retention-days: 30 + - uses: actions/upload-artifact@v4 + if: ${{ !cancelled() }} + with: + name: desktop-logs + path: ${{ env.APPDATA }}/ComfyUI/logs + retention-days: 30 From 6896f67e7b14757278bc77d8a42620c0565254e7 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 20:45:52 +1100 Subject: [PATCH 16/29] Add screenshot on app start --- tests/integration/appLifecycle.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/appLifecycle.spec.ts b/tests/integration/appLifecycle.spec.ts index 9c2f1b6a..333be620 100644 --- a/tests/integration/appLifecycle.spec.ts +++ b/tests/integration/appLifecycle.spec.ts @@ -5,6 +5,7 @@ import { test } from './autoCleaningTestApp'; 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' }); const getStartedButton = window.getByText('Get Started'); From 724a333e40268357b5bdd81a7d1859fbed40823d Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 21:22:04 +1100 Subject: [PATCH 17/29] nit - Add step names --- .github/workflows/integration_test_windows.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/integration_test_windows.yml b/.github/workflows/integration_test_windows.yml index 7666d3b3..96f61579 100644 --- a/.github/workflows/integration_test_windows.yml +++ b/.github/workflows/integration_test_windows.yml @@ -47,13 +47,15 @@ jobs: with: name: screenshot path: screenshot*.png - - uses: actions/upload-artifact@v4 + - name: Upload Test Results + uses: actions/upload-artifact@v4 if: ${{ !cancelled() }} with: name: test-results path: test-results/ retention-days: 30 - - uses: actions/upload-artifact@v4 + - name: Upload Desktop Logs + uses: actions/upload-artifact@v4 if: ${{ !cancelled() }} with: name: desktop-logs From a41d8d03db00e66350be8c05d3c2c8e1cc9c79bd Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 21:26:57 +1100 Subject: [PATCH 18/29] Copy log file location from runner shell to GH env --- .github/workflows/integration_test_windows.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/integration_test_windows.yml b/.github/workflows/integration_test_windows.yml index 96f61579..4ecffe5f 100644 --- a/.github/workflows/integration_test_windows.yml +++ b/.github/workflows/integration_test_windows.yml @@ -54,10 +54,12 @@ jobs: name: test-results path: test-results/ retention-days: 30 + - name: Copy desktop logs + run: echo ("COMFYUI_LOGS_DIR=" + $env:APPDATA + "/ComfyUI/logs/") >> $env:GITHUB_ENV - name: Upload Desktop Logs uses: actions/upload-artifact@v4 if: ${{ !cancelled() }} with: name: desktop-logs - path: ${{ env.APPDATA }}/ComfyUI/logs + path: ${{ env.COMFYUI_LOGS_DIR }} retention-days: 30 From 15000a464170a8153181229cb9b79e3fed817581 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 22:12:38 +1100 Subject: [PATCH 19/29] Fix GH action syntax --- .github/workflows/integration_test_windows.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/integration_test_windows.yml b/.github/workflows/integration_test_windows.yml index 4ecffe5f..a9802d37 100644 --- a/.github/workflows/integration_test_windows.yml +++ b/.github/workflows/integration_test_windows.yml @@ -54,8 +54,11 @@ jobs: name: test-results path: test-results/ retention-days: 30 - - name: Copy desktop logs - run: echo ("COMFYUI_LOGS_DIR=" + $env:APPDATA + "/ComfyUI/logs/") >> $env:GITHUB_ENV + - name: Copy desktop log location + if: ${{ !cancelled() }} + run: | + Write-Output "COMFYUI_LOGS_DIR=$env:APPDATA/ComfyUI/logs/" + Write-Output "COMFYUI_LOGS_DIR=$env:APPDATA/ComfyUI/logs/" >> $env:GITHUB_ENV - name: Upload Desktop Logs uses: actions/upload-artifact@v4 if: ${{ !cancelled() }} From b31e31e74ac1125e821344426d71299d38f38e3f Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 22:53:40 +1100 Subject: [PATCH 20/29] Attach desktop app logs before resetting appdata --- tests/integration/autoCleaningTestApp.ts | 7 ++++++- tests/integration/testEnvironment.ts | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/integration/autoCleaningTestApp.ts b/tests/integration/autoCleaningTestApp.ts index 002c1793..e2884163 100644 --- a/tests/integration/autoCleaningTestApp.ts +++ b/tests/integration/autoCleaningTestApp.ts @@ -4,10 +4,15 @@ import { TestApp } from './testApp'; import { TestEnvironment } from './testEnvironment'; export const test = testBase.extend<{ autoCleaningApp: AutoCleaningTestApp }>({ - autoCleaningApp: async ({}, use) => { + autoCleaningApp: async ({}, use, testInfo) => { // Launch Electron app. await using app = await AutoCleaningTestApp.create(); await use(app); + + // After test + const appEnv = app.testEnvironment; + await testInfo.attach('main.log', { path: appEnv.mainLogPath }); + await testInfo.attach('comfyui.log', { path: appEnv.comfyuiLogPath }); }, }); diff --git a/tests/integration/testEnvironment.ts b/tests/integration/testEnvironment.ts index 6eb8db64..effac877 100644 --- a/tests/integration/testEnvironment.ts +++ b/tests/integration/testEnvironment.ts @@ -1,4 +1,5 @@ import { rm } from 'node:fs/promises'; +import path from 'node:path'; import { getComfyUIAppDataPath } from 'tests/shared/utils'; import { TempDirectory } from './tempDirectory'; @@ -7,6 +8,14 @@ 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(); From 2fe6e1c2af3e4e82d81654a07d8aff4f9655a949 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 23:21:14 +1100 Subject: [PATCH 21/29] Fix throw when log missing --- tests/integration/autoCleaningTestApp.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/integration/autoCleaningTestApp.ts b/tests/integration/autoCleaningTestApp.ts index e2884163..322dbbd2 100644 --- a/tests/integration/autoCleaningTestApp.ts +++ b/tests/integration/autoCleaningTestApp.ts @@ -1,8 +1,15 @@ -import { test as testBase } from '@playwright/test'; +import { type TestInfo, test as testBase } 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 = testBase.extend<{ autoCleaningApp: AutoCleaningTestApp }>({ autoCleaningApp: async ({}, use, testInfo) => { // Launch Electron app. @@ -11,8 +18,8 @@ export const test = testBase.extend<{ autoCleaningApp: AutoCleaningTestApp }>({ // After test const appEnv = app.testEnvironment; - await testInfo.attach('main.log', { path: appEnv.mainLogPath }); - await testInfo.attach('comfyui.log', { path: appEnv.comfyuiLogPath }); + await attachIfExists(testInfo, appEnv.mainLogPath); + await attachIfExists(testInfo, appEnv.comfyuiLogPath); }, }); From 9440da9221e4687eedd164251ddd95d9e6b59ae8 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 23:30:00 +1100 Subject: [PATCH 22/29] Add app startup timeout --- tests/integration/appLifecycle.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/appLifecycle.spec.ts b/tests/integration/appLifecycle.spec.ts index 333be620..e6928a69 100644 --- a/tests/integration/appLifecycle.spec.ts +++ b/tests/integration/appLifecycle.spec.ts @@ -2,6 +2,8 @@ import { type Locator, expect } from '@playwright/test'; import { test } from './autoCleaningTestApp'; +const APP_START_TIMEOUT = process.env.CI ? 30_000 : 5000; + test.describe('App Lifecycle', () => { test('does all app startup things from previous test', async ({ autoCleaningApp }) => { const window = await autoCleaningApp.firstWindow(); @@ -9,7 +11,7 @@ test.describe('App Lifecycle', () => { const getStartedButton = window.getByText('Get Started'); - await expect(getStartedButton).toBeVisible(); + await expect(getStartedButton).toBeVisible({ timeout: APP_START_TIMEOUT }); await expect(getStartedButton).toBeEnabled(); await window.screenshot({ path: 'screenshot-load.png' }); From e18576f7baa6f259424f1079fd5312da3ed0b38e Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 23:46:31 +1100 Subject: [PATCH 23/29] Use debug log level in e2e tests --- .github/workflows/integration_test_windows.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/integration_test_windows.yml b/.github/workflows/integration_test_windows.yml index a9802d37..0237de2e 100644 --- a/.github/workflows/integration_test_windows.yml +++ b/.github/workflows/integration_test_windows.yml @@ -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 From 7cc1afeae6311cc2edab00919c6a09d6b07f56c5 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Wed, 12 Feb 2025 23:48:34 +1100 Subject: [PATCH 24/29] Add test screenshot --- tests/integration/appLifecycle.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/integration/appLifecycle.spec.ts b/tests/integration/appLifecycle.spec.ts index e6928a69..f613cdd0 100644 --- a/tests/integration/appLifecycle.spec.ts +++ b/tests/integration/appLifecycle.spec.ts @@ -9,9 +9,12 @@ test.describe('App Lifecycle', () => { const window = await autoCleaningApp.firstWindow(); await window.screenshot({ path: 'screenshot-app-start.png' }); + await window.waitForTimeout(APP_START_TIMEOUT); + await window.screenshot({ path: 'screenshot-app-start-timeout.png' }); + const getStartedButton = window.getByText('Get Started'); - await expect(getStartedButton).toBeVisible({ timeout: APP_START_TIMEOUT }); + await expect(getStartedButton).toBeVisible(); await expect(getStartedButton).toBeEnabled(); await window.screenshot({ path: 'screenshot-load.png' }); From f5e8719e56dff0f5c1d68651f066154021acb616 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Thu, 13 Feb 2025 00:20:58 +1100 Subject: [PATCH 25/29] Allow env vars to be passed into tested app --- tests/integration/testApp.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/testApp.ts b/tests/integration/testApp.ts index ba2cc978..14b578b7 100644 --- a/tests/integration/testApp.ts +++ b/tests/integration/testApp.ts @@ -49,7 +49,6 @@ export class TestApp implements AsyncDisposable { args: ['.'], executablePath, cwd: '.', - env: {}, }); await localTestQoL(app); return app; From f13f409987c96e7afd2065fd680a7e2d9218c29d Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Thu, 13 Feb 2025 00:49:01 +1100 Subject: [PATCH 26/29] Prefer Playwright attach to GH action upload --- .github/workflows/integration_test_windows.yml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/.github/workflows/integration_test_windows.yml b/.github/workflows/integration_test_windows.yml index 0237de2e..98a373a6 100644 --- a/.github/workflows/integration_test_windows.yml +++ b/.github/workflows/integration_test_windows.yml @@ -55,15 +55,3 @@ jobs: name: test-results path: test-results/ retention-days: 30 - - name: Copy desktop log location - if: ${{ !cancelled() }} - run: | - Write-Output "COMFYUI_LOGS_DIR=$env:APPDATA/ComfyUI/logs/" - Write-Output "COMFYUI_LOGS_DIR=$env:APPDATA/ComfyUI/logs/" >> $env:GITHUB_ENV - - name: Upload Desktop Logs - uses: actions/upload-artifact@v4 - if: ${{ !cancelled() }} - with: - name: desktop-logs - path: ${{ env.COMFYUI_LOGS_DIR }} - retention-days: 30 From 95f382a19792af19fdf18b926a46d86f609df59c Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Thu, 13 Feb 2025 00:51:58 +1100 Subject: [PATCH 27/29] nit --- tests/integration/autoCleaningTestApp.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/autoCleaningTestApp.ts b/tests/integration/autoCleaningTestApp.ts index 322dbbd2..c21c78e9 100644 --- a/tests/integration/autoCleaningTestApp.ts +++ b/tests/integration/autoCleaningTestApp.ts @@ -1,4 +1,4 @@ -import { type TestInfo, test as testBase } from '@playwright/test'; +import { type TestInfo, test as baseTest } from '@playwright/test'; import { pathExists } from 'tests/shared/utils'; import { TestApp } from './testApp'; @@ -10,7 +10,7 @@ async function attachIfExists(testInfo: TestInfo, path: string) { } } -export const test = testBase.extend<{ autoCleaningApp: AutoCleaningTestApp }>({ +export const test = baseTest.extend<{ autoCleaningApp: AutoCleaningTestApp }>({ autoCleaningApp: async ({}, use, testInfo) => { // Launch Electron app. await using app = await AutoCleaningTestApp.create(); From 6271ff46399e1469bef2163cc5a86d25ee6b1b95 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Thu, 13 Feb 2025 00:56:30 +1100 Subject: [PATCH 28/29] Remove debug code --- tests/integration/appLifecycle.spec.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/integration/appLifecycle.spec.ts b/tests/integration/appLifecycle.spec.ts index f613cdd0..e6928a69 100644 --- a/tests/integration/appLifecycle.spec.ts +++ b/tests/integration/appLifecycle.spec.ts @@ -9,12 +9,9 @@ test.describe('App Lifecycle', () => { const window = await autoCleaningApp.firstWindow(); await window.screenshot({ path: 'screenshot-app-start.png' }); - await window.waitForTimeout(APP_START_TIMEOUT); - await window.screenshot({ path: 'screenshot-app-start-timeout.png' }); - const getStartedButton = window.getByText('Get Started'); - await expect(getStartedButton).toBeVisible(); + await expect(getStartedButton).toBeVisible({ timeout: APP_START_TIMEOUT }); await expect(getStartedButton).toBeEnabled(); await window.screenshot({ path: 'screenshot-load.png' }); From 055b899258fb45dee11f1c761ae121562a995c5d Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Thu, 13 Feb 2025 01:05:38 +1100 Subject: [PATCH 29/29] nit --- tests/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/README.md b/tests/README.md index e0ad8e94..7006b9a3 100644 --- a/tests/README.md +++ b/tests/README.md @@ -14,11 +14,11 @@ yarn run test:unit 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. -These tests are designed to be run in CI or a virtual machine. - ### Running ```bash