From e5c9d1e39fd0e4bb30e25d69adba2cc7ae3f0071 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 23 Feb 2022 13:55:41 -0700 Subject: [PATCH] chore: best-effort cleanup for output folders that are mounted (#12300) Fixes #12106 --- package-lock.json | 2 -- packages/playwright-core/src/utils/utils.ts | 28 ++++++++++++++++++-- packages/playwright-test/package.json | 1 - packages/playwright-test/src/runner.ts | 6 ++--- packages/playwright-test/src/workerRunner.ts | 7 ++--- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index 34bbfbd798..d4a8601aba 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 7be6c8b27a..ca8feb558f 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,32 @@ 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 => { + 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. + try { + const entries = await readDirAsync(dir).catch(e => []); + await Promise.all(entries.map(entry => removeFolderAsync(path.join(dir, entry)))); + fulfill(undefined); + } catch (e) { + fulfill(e); + } + } else { + fulfill(error ?? undefined); + } }); }); })); diff --git a/packages/playwright-test/package.json b/packages/playwright-test/package.json index 505b12cd79..0a89250f32 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 94e40a0a7e..c7b6c588ab 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 19b39884c1..bd8b5f909a 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, 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; @@ -314,7 +311,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) {