From e1f287f2558ad7c8a484c7fbdac6d189cb8f49e0 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 9 Feb 2023 16:03:54 -0800 Subject: [PATCH] chore: more watch tests (#20797) --- .../playwright-test/src/runner/watchMode.ts | 119 +++++++++++------- .../playwright-test-fixtures.ts | 16 +-- tests/playwright-test/reporter-html.spec.ts | 4 +- tests/playwright-test/watch.spec.ts | 86 ++++++++++++- 4 files changed, 169 insertions(+), 56 deletions(-) diff --git a/packages/playwright-test/src/runner/watchMode.ts b/packages/playwright-test/src/runner/watchMode.ts index 557495df2d..df1ca74082 100644 --- a/packages/playwright-test/src/runner/watchMode.ts +++ b/packages/playwright-test/src/runner/watchMode.ts @@ -18,7 +18,7 @@ import readline from 'readline'; import { createGuid, ManualPromise } from 'playwright-core/lib/utils'; import type { FullConfigInternal, FullProjectInternal } from '../common/types'; import { Multiplexer } from '../reporters/multiplexer'; -import { createFileMatcherFromArguments } from '../util'; +import { createFileMatcher, createFileMatcherFromArguments } from '../util'; import type { Matcher } from '../util'; import { createTaskRunnerForWatch, createTaskRunnerForWatchSetup } from './tasks'; import type { TaskRunnerState } from './tasks'; @@ -26,6 +26,7 @@ import { buildProjectsClosure, filterProjects } from './projectUtils'; import { clearCompilationCache, collectAffectedTestFiles } from '../common/compilationCache'; import type { FullResult } from 'packages/playwright-test/reporter'; import { chokidar } from '../utilsBundle'; +import type { FSWatcher as CFSWatcher } from 'chokidar'; import { createReporter } from './reporters'; import { colors } from 'playwright-core/lib/utilsBundle'; import { enquirer } from '../utilsBundle'; @@ -34,32 +35,74 @@ import { PlaywrightServer } from 'playwright-core/lib/remote/playwrightServer'; import ListReporter from '../reporters/list'; class FSWatcher { - private _dirtyFiles = new Set(); + private _dirtyTestFiles = new Map>(); private _notifyDirtyFiles: (() => void) | undefined; + private _watcher: CFSWatcher | undefined; + private _timer: NodeJS.Timeout | undefined; - constructor(dirs: string[]) { - let timer: NodeJS.Timer; - chokidar.watch(dirs, { ignoreInitial: true }).on('all', async (event, file) => { + async update(config: FullConfigInternal) { + const commandLineFileMatcher = config._internal.cliArgs.length ? createFileMatcherFromArguments(config._internal.cliArgs) : () => true; + const projects = filterProjects(config.projects, config._internal.cliProjectFilter); + const projectClosure = buildProjectsClosure(projects); + const projectFilters = new Map(); + for (const project of projectClosure) { + const testMatch = createFileMatcher(project.testMatch); + const testIgnore = createFileMatcher(project.testIgnore); + projectFilters.set(project, file => { + if (!file.startsWith(project.testDir) || !testMatch(file) || testIgnore(file)) + return false; + return project._internal.type === 'dependency' || commandLineFileMatcher(file); + }); + } + + if (this._timer) + clearTimeout(this._timer); + if (this._watcher) + await this._watcher.close(); + + this._watcher = chokidar.watch(projectClosure.map(p => p.testDir), { ignoreInitial: true }).on('all', async (event, file) => { if (event !== 'add' && event !== 'change') return; - this._dirtyFiles.add(file); - if (timer) - clearTimeout(timer); - timer = setTimeout(() => { + + const testFiles = new Set(); + collectAffectedTestFiles(file, testFiles); + const testFileArray = [...testFiles]; + + let hasMatches = false; + for (const [project, filter] of projectFilters) { + const filteredFiles = testFileArray.filter(filter); + if (!filteredFiles.length) + continue; + let set = this._dirtyTestFiles.get(project); + if (!set) { + set = new Set(); + this._dirtyTestFiles.set(project, set); + } + filteredFiles.map(f => set!.add(f)); + hasMatches = true; + } + + if (!hasMatches) + return; + + if (this._timer) + clearTimeout(this._timer); + this._timer = setTimeout(() => { this._notifyDirtyFiles?.(); }, 250); }); + } - async onDirtyFiles(): Promise { - if (this._dirtyFiles.size) + async onDirtyTestFiles(): Promise { + if (this._dirtyTestFiles.size) return; await new Promise(f => this._notifyDirtyFiles = f); } - takeDirtyFiles(): Set { - const result = this._dirtyFiles; - this._dirtyFiles = new Set(); + takeDirtyTestFiles(): Map> { + const result = this._dirtyTestFiles; + this._dirtyTestFiles = new Map(); return result; } } @@ -84,13 +127,12 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise(); const originalWorkers = config.workers; - const fsWatcher = new FSWatcher(projectClosure.map(p => p.testDir)); + const fsWatcher = new FSWatcher(); + await fsWatcher.update(config); - let lastRun: { type: 'changed' | 'regular' | 'failed', failedTestIds?: Set, dirtyFiles?: Set } = { type: 'regular' }; + let lastRun: { type: 'changed' | 'regular' | 'failed', failedTestIds?: Set, dirtyTestFiles?: Map> } = { type: 'regular' }; let result: FullResult['status'] = 'passed'; // Enter the watch loop. @@ -100,7 +142,7 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise lastRun.failedTestIds!.has(id); await runTests(config, failedTestIdCollector, { title: 're-running tests' }); @@ -211,30 +257,15 @@ export async function runWatchModeLoop(config: FullConfigInternal): Promise, projectClosure: FullProjectInternal[], changedFiles: Set, title?: string) { - const commandLineFileMatcher = config._internal.cliArgs.length ? createFileMatcherFromArguments(config._internal.cliArgs) : () => true; - - // Resolve files that depend on the changed files. +async function runChangedTests(config: FullConfigInternal, failedTestIdCollector: Set, filesByProject: Map>, title?: string) { const testFiles = new Set(); - for (const file of changedFiles) - collectAffectedTestFiles(file, testFiles); - - // Collect projects with changes. - const filesByProject = new Map(); - for (const project of projectClosure) { - const projectFiles: string[] = []; - for (const file of testFiles) { - if (!file.startsWith(project.testDir)) - continue; - if (project._internal.type === 'dependency' || commandLineFileMatcher(file)) - projectFiles.push(file); - } - if (projectFiles.length) - filesByProject.set(project, projectFiles); - } + for (const files of filesByProject.values()) + files.forEach(f => testFiles.add(f)); // Collect all the affected projects, follow project dependencies. // Prepare to exclude all the projects that do not depend on this file, as if they did not exist. + const projects = filterProjects(config.projects, config._internal.cliProjectFilter); + const projectClosure = buildProjectsClosure(projects); const affectedProjects = affectedProjectsClosure(projectClosure, [...filesByProject.keys()]); const affectsAnyDependency = [...affectedProjects].some(p => p._internal.type === 'dependency'); const projectsToIgnore = new Set(projectClosure.filter(p => !affectedProjects.has(p))); @@ -242,7 +273,7 @@ async function runChangedTests(config: FullConfigInternal, failedTestIdCollector // If there are affected dependency projects, do the full run, respect the original CLI. // if there are no affected dependency projects, intersect CLI with dirty files const additionalFileMatcher = affectsAnyDependency ? () => true : (file: string) => testFiles.has(file); - return await runTests(config, failedTestIdCollector, { projectsToIgnore, additionalFileMatcher, title: title || 'files changed' }); + await runTests(config, failedTestIdCollector, { projectsToIgnore, additionalFileMatcher, title: title || 'files changed' }); } async function runTests(config: FullConfigInternal, failedTestIdCollector: Set, options?: { diff --git a/tests/playwright-test/playwright-test-fixtures.ts b/tests/playwright-test/playwright-test-fixtures.ts index 41e79646f5..477c3a7d05 100644 --- a/tests/playwright-test/playwright-test-fixtures.ts +++ b/tests/playwright-test/playwright-test-fixtures.ts @@ -57,7 +57,7 @@ type TSCResult = { type Files = { [key: string]: string | Buffer }; type Params = { [key: string]: string | number | boolean | string[] }; -async function writeFiles(testInfo: TestInfo, files: Files) { +async function writeFiles(testInfo: TestInfo, files: Files, initial: boolean) { const baseDir = testInfo.outputPath(); const headerJS = ` @@ -71,7 +71,7 @@ async function writeFiles(testInfo: TestInfo, files: Files) { `; const hasConfig = Object.keys(files).some(name => name.includes('.config.')); - if (!hasConfig) { + if (initial && !hasConfig) { files = { ...files, 'playwright.config.ts': ` @@ -79,7 +79,7 @@ async function writeFiles(testInfo: TestInfo, files: Files) { `, }; } - if (!Object.keys(files).some(name => name.includes('package.json'))) { + if (initial && !Object.keys(files).some(name => name.includes('package.json'))) { files = { ...files, 'package.json': `{ "name": "test-project" }`, @@ -280,13 +280,13 @@ export const test = base .extend(serverFixtures) .extend({ writeFiles: async ({}, use, testInfo) => { - await use(files => writeFiles(testInfo, files)); + await use(files => writeFiles(testInfo, files, false)); }, runInlineTest: async ({ childProcess }, use, testInfo: TestInfo) => { const cacheDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'playwright-test-cache-')); await use(async (files: Files, params: Params = {}, env: NodeJS.ProcessEnv = {}, options: RunOptions = {}, beforeRunPlaywrightTest?: ({ baseDir }: { baseDir: string }) => Promise) => { - const baseDir = await writeFiles(testInfo, files); + const baseDir = await writeFiles(testInfo, files, true); if (beforeRunPlaywrightTest) await beforeRunPlaywrightTest({ baseDir }); return await runPlaywrightTest(childProcess, baseDir, params, { ...env, PWTEST_CACHE_DIR: cacheDir }, options); @@ -298,7 +298,7 @@ export const test = base const cacheDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'playwright-test-cache-')); let testProcess: TestChildProcess | undefined; await use(async (files: Files, env: NodeJS.ProcessEnv = {}, options: RunOptions = {}) => { - const baseDir = await writeFiles(testInfo, files); + const baseDir = await writeFiles(testInfo, files, true); testProcess = watchPlaywrightTest(childProcess, baseDir, { ...env, PWTEST_CACHE_DIR: cacheDir }, options); return testProcess; }); @@ -308,14 +308,14 @@ export const test = base runCommand: async ({ childProcess }, use, testInfo: TestInfo) => { await use(async (files: Files, args: string[]) => { - const baseDir = await writeFiles(testInfo, files); + const baseDir = await writeFiles(testInfo, files, true); return await runPlaywrightCommand(childProcess, baseDir, args, { }); }); }, runTSC: async ({ childProcess }, use, testInfo) => { await use(async files => { - const baseDir = await writeFiles(testInfo, { 'tsconfig.json': JSON.stringify(TSCONFIG), ...files }); + const baseDir = await writeFiles(testInfo, { 'tsconfig.json': JSON.stringify(TSCONFIG), ...files }, true); const tsc = childProcess({ command: ['npx', 'tsc', '-p', baseDir], cwd: baseDir, diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts index 53b4441cca..c89b5ac094 100644 --- a/tests/playwright-test/reporter-html.spec.ts +++ b/tests/playwright-test/reporter-html.spec.ts @@ -18,7 +18,7 @@ import fs from 'fs'; import path from 'path'; import url from 'url'; import { test as baseTest, expect, createImage } from './playwright-test-fixtures'; -import type { HttpServer } from '../../packages/playwright-core/lib/utils'; +import type { HttpServer } from '../../packages/playwright-core/src/utils'; import { startHtmlReportServer } from '../../packages/playwright-test/lib/reporters/html'; import { spawnAsync } from 'playwright-core/lib/utils'; @@ -27,7 +27,7 @@ const test = baseTest.extend<{ showReport: (reportFolder?: string) => Promise { reportFolder ??= testInfo.outputPath('playwright-report'); - server = startHtmlReportServer(reportFolder); + server = startHtmlReportServer(reportFolder) as HttpServer; const location = await server.start(); await page.goto(location); }); diff --git a/tests/playwright-test/watch.spec.ts b/tests/playwright-test/watch.spec.ts index dc3c111aee..43f017dcda 100644 --- a/tests/playwright-test/watch.spec.ts +++ b/tests/playwright-test/watch.spec.ts @@ -395,13 +395,95 @@ test('should not trigger on changes to non-tests', async ({ runWatchTest, writeF await testProcess.waitForOutput('a.test.ts:5:11 › passes'); await testProcess.waitForOutput('b.test.ts:5:11 › passes'); await testProcess.waitForOutput('Waiting for file changes.'); + testProcess.clearOutput(); writeFiles({ 'helper.ts': ` console.log('helper'); `, }); + await new Promise(f => setTimeout(f, 1000)); - expect(testProcess.output).not.toContain('a.test.ts'); - expect(testProcess.output).not.toContain('b.test.ts'); + expect(testProcess.output).not.toContain('Waiting for file changes.'); +}); + +test('should only watch selected projects', async ({ runWatchTest, writeFiles }) => { + const testProcess = await runWatchTest({ + 'playwright.config.ts': ` + import { defineConfig } from '@playwright/test'; + export default defineConfig({ projects: [{name: 'foo'}, {name: 'bar'}] }); + `, + 'a.test.ts': ` + pwt.test('passes', () => {}); + `, + }, {}, { additionalArgs: ['--project=foo'] }); + await testProcess.waitForOutput('npx playwright test --project foo'); + await testProcess.waitForOutput('[foo] › a.test.ts:5:11 › passes'); + expect(testProcess.output).not.toContain('[bar]'); + await testProcess.waitForOutput('Waiting for file changes.'); + + testProcess.clearOutput(); + writeFiles({ + 'a.test.ts': ` + pwt.test('passes', () => {}); + `, + }); + + await testProcess.waitForOutput('npx playwright test --project foo'); + await testProcess.waitForOutput('[foo] › a.test.ts:5:11 › passes'); + await testProcess.waitForOutput('Waiting for file changes.'); + expect(testProcess.output).not.toContain('[bar]'); +}); + +test('should watch filtered files', async ({ runWatchTest, writeFiles }) => { + const testProcess = await runWatchTest({ + 'a.test.ts': ` + pwt.test('passes', () => {}); + `, + 'b.test.ts': ` + pwt.test('passes', () => {}); + `, + }, {}, { additionalArgs: ['a.test.ts'] }); + await testProcess.waitForOutput('npx playwright test a.test.ts'); + await testProcess.waitForOutput('a.test.ts:5:11 › passes'); + expect(testProcess.output).not.toContain('b.test'); + await testProcess.waitForOutput('Waiting for file changes.'); + + testProcess.clearOutput(); + writeFiles({ + 'b.test.ts': ` + pwt.test('passes', () => {}); + `, + }); + + await new Promise(f => setTimeout(f, 1000)); + expect(testProcess.output).not.toContain('Waiting for file changes.'); +}); + +test('should not watch unfiltered files', async ({ runWatchTest, writeFiles }) => { + const testProcess = await runWatchTest({ + 'a.test.ts': ` + pwt.test('passes', () => {}); + `, + 'b.test.ts': ` + pwt.test('passes', () => {}); + `, + }, {}, { additionalArgs: ['a.test.ts'] }); + await testProcess.waitForOutput('npx playwright test a.test.ts'); + await testProcess.waitForOutput('a.test.ts:5:11 › passes'); + expect(testProcess.output).not.toContain('b.test'); + await testProcess.waitForOutput('Waiting for file changes.'); + + testProcess.clearOutput(); + writeFiles({ + 'a.test.ts': ` + pwt.test('passes', () => {}); + `, + }); + + testProcess.clearOutput(); + await testProcess.waitForOutput('npx playwright test a.test.ts (files changed)'); + await testProcess.waitForOutput('a.test.ts:5:11 › passes'); + expect(testProcess.output).not.toContain('b.test'); + await testProcess.waitForOutput('Waiting for file changes.'); });