From 1c11203e9840af2fbc6f9649d6fd223a869281b7 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 23 Feb 2022 15:03:27 -0700 Subject: [PATCH] Revert "cherry-pick(#12300): chore: best-effort cleanup for output folders that are mounted (#12318)" This reverts commit 0a1c1dad6769ed23ca808936c1d43645a6778a45. Reason for revert: turns out this fix results in a 5-second delay when starting tests in docker, with `test-results` folder being a non-removable mount. The reason for the delay is the `maxBusyTries` option that we supply by default to rimraf when trying to remove the folder. While this option might come handy when removing temporary browser profile folder, it doesn't serve us well in this particular usecase. References #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, 14 insertions(+), 30 deletions(-) diff --git a/package-lock.json b/package-lock.json index d1acd3d4d2e81..373d7eafb2d15 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7631,6 +7631,7 @@ "pixelmatch": "5.2.1", "playwright-core": "1.19.1", "pngjs": "6.0.0", + "rimraf": "3.0.2", "source-map-support": "0.4.18", "stack-utils": "2.0.5", "yazl": "2.5.1" @@ -8447,6 +8448,7 @@ "pixelmatch": "5.2.1", "playwright-core": "1.19.1", "pngjs": "6.0.0", + "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 764eb0c5cd05f..7bc5c94c3f43d 100644 --- a/packages/playwright-core/src/utils/utils.ts +++ b/packages/playwright-core/src/utils/utils.ts @@ -21,7 +21,6 @@ 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'; @@ -30,8 +29,6 @@ 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. @@ -418,32 +415,11 @@ 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 }, 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); - } + removeFolder(dir, { maxBusyTries: 10 }, error => { + fulfill(error ?? undefined); }); }); })); diff --git a/packages/playwright-test/package.json b/packages/playwright-test/package.json index edddbc04bf3c3..4cffc31575fb4 100644 --- a/packages/playwright-test/package.json +++ b/packages/playwright-test/package.json @@ -61,6 +61,7 @@ "pixelmatch": "5.2.1", "playwright-core": "1.19.1", "pngjs": "6.0.0", + "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 bd2d94b551500..17dcd19a1f4a8 100644 --- a/packages/playwright-test/src/runner.ts +++ b/packages/playwright-test/src/runner.ts @@ -15,6 +15,7 @@ * limitations under the License. */ +import rimraf from 'rimraf'; import * as fs from 'fs'; import * as path from 'path'; import { promisify } from 'util'; @@ -37,8 +38,9 @@ import { Minimatch } from 'minimatch'; import { Config, FullConfig } from './types'; import { WebServer } from './webServer'; import { raceAgainstTimeout } from 'playwright-core/lib/utils/async'; -import { removeFoldersOrDie, SigIntWatcher } from 'playwright-core/lib/utils/utils'; +import { 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']; @@ -350,7 +352,7 @@ export class Runner { // 12. Remove output directores. try { - await removeFoldersOrDie(Array.from(outputDirs)); + await Promise.all(Array.from(outputDirs).map(outputDir => removeFolderAsync(outputDir))); } 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 ed6e3f1f3fb36..f421ade8b7e06 100644 --- a/packages/playwright-test/src/workerRunner.ts +++ b/packages/playwright-test/src/workerRunner.ts @@ -14,6 +14,8 @@ * 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'; @@ -25,9 +27,10 @@ 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; @@ -305,7 +308,7 @@ export class WorkerRunner extends EventEmitter { const preserveOutput = this._loader.fullConfig().preserveOutput === 'always' || (this._loader.fullConfig().preserveOutput === 'failures-only' && isFailure); if (!preserveOutput) - await removeFolders([testInfo.outputDir]); + await removeFolderAsync(testInfo.outputDir).catch(e => {}); } private async _runTestWithBeforeHooks(test: TestCase, testInfo: TestInfoImpl) {