From df7fbe785799415140431b1561632c2d1b496c32 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 23 Feb 2022 15:10:11 -0700 Subject: [PATCH] Revert "chore: best-effort cleanup for output folders that are mounted (#12300)" (#12321) This reverts commit e5c9d1e39fd0e4bb30e25d69adba2cc7ae3f0071. 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 d4a8601aba..34bbfbd798 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7525,6 +7525,7 @@ "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" @@ -8340,6 +8341,7 @@ "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 ca8feb558f..7be6c8b27a 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 0a89250f32..505b12cd79 100644 --- a/packages/playwright-test/package.json +++ b/packages/playwright-test/package.json @@ -58,6 +58,7 @@ "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 c7b6c588ab..94e40a0a7e 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 bd8b5f909a..19b39884c1 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, 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; @@ -311,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 removeFolders([testInfo.outputDir]); + await removeFolderAsync(testInfo.outputDir).catch(e => {}); } private async _runTestWithBeforeHooks(test: TestCase, testInfo: TestInfoImpl) {