From ec7be06627f3b7c5bded5e9853d74c6682585f73 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Tue, 22 Feb 2022 22:41:44 -0700 Subject: [PATCH 1/6] chore: best-effort cleanup for output folders that are mounted Fixes #12106 --- package-lock.json | 2 -- packages/playwright-core/src/utils/utils.ts | 24 ++++++++++++++++++-- packages/playwright-test/package.json | 1 - packages/playwright-test/src/runner.ts | 6 ++--- packages/playwright-test/src/workerRunner.ts | 7 ++---- 5 files changed, 26 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index 34bbfbd798f5e..d4a8601aba672 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7525,7 +7525,6 @@ "open": "8.4.0", "pirates": "4.0.4", "playwright-core": "1.20.0-next", - "rimraf": "3.0.2", "source-map-support": "0.4.18", "stack-utils": "2.0.5", "yazl": "2.5.1" @@ -8341,7 +8340,6 @@ "open": "8.4.0", "pirates": "4.0.4", "playwright-core": "1.20.0-next", - "rimraf": "3.0.2", "source-map-support": "0.4.18", "stack-utils": "2.0.5", "yazl": "2.5.1" diff --git a/packages/playwright-core/src/utils/utils.ts b/packages/playwright-core/src/utils/utils.ts index 7be6c8b27a25e..d5d3d535de770 100644 --- a/packages/playwright-core/src/utils/utils.ts +++ b/packages/playwright-core/src/utils/utils.ts @@ -21,6 +21,7 @@ import removeFolder from 'rimraf'; import * as crypto from 'crypto'; import os from 'os'; import http from 'http'; +import { promisify } from 'util'; import https from 'https'; import { spawn, SpawnOptions, execSync } from 'child_process'; import { getProxyForUrl } from 'proxy-from-env'; @@ -29,6 +30,8 @@ import { getUbuntuVersionSync, parseOSReleaseText } from './ubuntuVersion'; import { NameValue } from '../protocol/channels'; import ProgressBar from 'progress'; +const removeFolderAsync = promisify(removeFolder); +const readDirAsync = promisify(fs.readdir); // `https-proxy-agent` v5 is written in TypeScript and exposes generated types. // However, as of June 2020, its types are generated with tsconfig that enables // `esModuleInterop` option. @@ -415,11 +418,28 @@ export function createGuid(): string { return crypto.randomBytes(16).toString('hex'); } +export async function removeFoldersOrDie(dirs: string[]): Promise { + const errors = await removeFolders(dirs); + for (const error of errors) { + if (error) + throw error; + } +} + export async function removeFolders(dirs: string[]): Promise> { return await Promise.all(dirs.map((dir: string) => { return new Promise(fulfill => { - removeFolder(dir, { maxBusyTries: 10 }, error => { - fulfill(error ?? undefined); + removeFolder(dir, { maxBusyTries: 10 }, async error => { + // We failed to remove folder, might be due to the whole folder being mounted inside a container: + // https://github.com/microsoft/playwright/issues/12106 + // Do a best-effort to remove all files inside of it instead. + try { + const entries = await readDirAsync(dir); + await Promise.all(entries.map(entry => removeFolderAsync(path.join(dir, entry)))); + fulfill(undefined); + } catch (e) { + fulfill(e); + } }); }); })); diff --git a/packages/playwright-test/package.json b/packages/playwright-test/package.json index 505b12cd79ef6..0a89250f3202f 100644 --- a/packages/playwright-test/package.json +++ b/packages/playwright-test/package.json @@ -58,7 +58,6 @@ "open": "8.4.0", "pirates": "4.0.4", "playwright-core": "1.20.0-next", - "rimraf": "3.0.2", "source-map-support": "0.4.18", "stack-utils": "2.0.5", "yazl": "2.5.1" diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts index 94e40a0a7eef0..c7b6c588ab6ee 100644 --- a/packages/playwright-test/src/runner.ts +++ b/packages/playwright-test/src/runner.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import rimraf from 'rimraf'; import * as fs from 'fs'; import * as path from 'path'; import { promisify } from 'util'; @@ -38,9 +37,8 @@ import { Minimatch } from 'minimatch'; import { Config, FullConfig } from './types'; import { WebServer } from './webServer'; import { raceAgainstTimeout } from 'playwright-core/lib/utils/async'; -import { SigIntWatcher } from 'playwright-core/lib/utils/utils'; +import { removeFoldersOrDie, SigIntWatcher } from 'playwright-core/lib/utils/utils'; -const removeFolderAsync = promisify(rimraf); const readDirAsync = promisify(fs.readdir); const readFileAsync = promisify(fs.readFile); export const kDefaultConfigFiles = ['playwright.config.ts', 'playwright.config.js', 'playwright.config.mjs']; @@ -352,7 +350,7 @@ export class Runner { // 12. Remove output directores. try { - await Promise.all(Array.from(outputDirs).map(outputDir => removeFolderAsync(outputDir))); + await removeFoldersOrDie(Array.from(outputDirs)); } catch (e) { this._reporter.onError?.(serializeError(e)); return { status: 'failed' }; diff --git a/packages/playwright-test/src/workerRunner.ts b/packages/playwright-test/src/workerRunner.ts index 07c9716146c22..0a448f7db2422 100644 --- a/packages/playwright-test/src/workerRunner.ts +++ b/packages/playwright-test/src/workerRunner.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -import rimraf from 'rimraf'; -import util from 'util'; import colors from 'colors/safe'; import { EventEmitter } from 'events'; import { serializeError, prependToTestError, formatLocation } from './util'; @@ -27,10 +25,9 @@ import { Annotations, TestError, TestInfo, TestStepInternal, WorkerInfo } from ' import { ProjectImpl } from './project'; import { FixtureRunner } from './fixtures'; import { raceAgainstTimeout } from 'playwright-core/lib/utils/async'; +import { removeFolders } from 'playwright-core/lib/utils/utils'; import { TestInfoImpl } from './testInfo'; -const removeFolderAsync = util.promisify(rimraf); - export class WorkerRunner extends EventEmitter { private _params: WorkerInitParams; private _loader!: Loader; @@ -317,7 +314,7 @@ export class WorkerRunner extends EventEmitter { const preserveOutput = this._loader.fullConfig().preserveOutput === 'always' || (this._loader.fullConfig().preserveOutput === 'failures-only' && isFailure); if (!preserveOutput) - await removeFolderAsync(testInfo.outputDir).catch(e => {}); + await removeFolders([testInfo.outputDir]); } private async _runTestWithBeforeHooks(test: TestCase, testInfo: TestInfoImpl) { From 1a44ad2e26392b0562b775486041fff77e1b089c Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 23 Feb 2022 09:43:45 -0700 Subject: [PATCH 2/6] fix --- packages/playwright-core/src/utils/utils.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/playwright-core/src/utils/utils.ts b/packages/playwright-core/src/utils/utils.ts index d5d3d535de770..b6608ed193172 100644 --- a/packages/playwright-core/src/utils/utils.ts +++ b/packages/playwright-core/src/utils/utils.ts @@ -430,6 +430,10 @@ export async function removeFolders(dirs: string[]): Promise { return new Promise(fulfill => { removeFolder(dir, { maxBusyTries: 10 }, async error => { + if (!error) { + fulfill(undefined); + return; + } // We failed to remove folder, might be due to the whole folder being mounted inside a container: // https://github.com/microsoft/playwright/issues/12106 // Do a best-effort to remove all files inside of it instead. From e6d0eba412d88c3f44d1ace65f9f3d67eaa91480 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 23 Feb 2022 11:27:09 -0700 Subject: [PATCH 3/6] more narrow scope --- packages/playwright-core/src/utils/utils.ts | 26 ++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/playwright-core/src/utils/utils.ts b/packages/playwright-core/src/utils/utils.ts index b6608ed193172..549abf4e122d5 100644 --- a/packages/playwright-core/src/utils/utils.ts +++ b/packages/playwright-core/src/utils/utils.ts @@ -430,19 +430,19 @@ export async function removeFolders(dirs: string[]): Promise { return new Promise(fulfill => { removeFolder(dir, { maxBusyTries: 10 }, async error => { - if (!error) { - fulfill(undefined); - return; - } - // We failed to remove folder, might be due to the whole folder being mounted inside a container: - // https://github.com/microsoft/playwright/issues/12106 - // Do a best-effort to remove all files inside of it instead. - try { - const entries = await readDirAsync(dir); - await Promise.all(entries.map(entry => removeFolderAsync(path.join(dir, entry)))); - fulfill(undefined); - } catch (e) { - fulfill(e); + if (error?.code === 'EBUSY') { + // We failed to remove folder, might be due to the whole folder being mounted inside a container: + // https://github.com/microsoft/playwright/issues/12106 + // Do a best-effort to remove all files inside of it instead. + try { + const entries = await readDirAsync(dir); + await Promise.all(entries.map(entry => removeFolderAsync(path.join(dir, entry)))); + fulfill(undefined); + } catch (e) { + fulfill(e); + } + } else { + fulfill(error); } }); }); From b7e4391387c2731c01e68c48fce41f69fe79c1c9 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 23 Feb 2022 11:43:40 -0700 Subject: [PATCH 4/6] check ebusy --- packages/playwright-core/src/utils/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/playwright-core/src/utils/utils.ts b/packages/playwright-core/src/utils/utils.ts index 549abf4e122d5..958a7d1668905 100644 --- a/packages/playwright-core/src/utils/utils.ts +++ b/packages/playwright-core/src/utils/utils.ts @@ -430,7 +430,7 @@ export async function removeFolders(dirs: string[]): Promise { return new Promise(fulfill => { removeFolder(dir, { maxBusyTries: 10 }, async error => { - if (error?.code === 'EBUSY') { + if ((error as any)?.code === 'EBUSY') { // We failed to remove folder, might be due to the whole folder being mounted inside a container: // https://github.com/microsoft/playwright/issues/12106 // Do a best-effort to remove all files inside of it instead. From 6271463f32d85a3fb5a1a7a435721aabdc4d4615 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 23 Feb 2022 11:48:56 -0700 Subject: [PATCH 5/6] address comments --- packages/playwright-core/src/utils/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/playwright-core/src/utils/utils.ts b/packages/playwright-core/src/utils/utils.ts index 958a7d1668905..68651598e513e 100644 --- a/packages/playwright-core/src/utils/utils.ts +++ b/packages/playwright-core/src/utils/utils.ts @@ -442,7 +442,7 @@ export async function removeFolders(dirs: string[]): Promise Date: Wed, 23 Feb 2022 12:01:00 -0700 Subject: [PATCH 6/6] another comments --- packages/playwright-core/src/utils/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/playwright-core/src/utils/utils.ts b/packages/playwright-core/src/utils/utils.ts index 68651598e513e..ca8feb558fe5c 100644 --- a/packages/playwright-core/src/utils/utils.ts +++ b/packages/playwright-core/src/utils/utils.ts @@ -435,7 +435,7 @@ export async function removeFolders(dirs: string[]): Promise []); await Promise.all(entries.map(entry => removeFolderAsync(path.join(dir, entry)))); fulfill(undefined); } catch (e) {