From ebc52d10e4ddc846b142a0e233934dd6fc2676dc Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 23 Feb 2022 15:00:38 -0700 Subject: [PATCH] Revert "chore: best-effort cleanup for output folders that are mounted (#12300)" 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. --- 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 d4a8601aba672..34bbfbd798f5e 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 ca8feb558fe5c..7be6c8b27a25e 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 0a89250f3202f..505b12cd79ef6 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 c7b6c588ab6ee..94e40a0a7eef0 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 bd8b5f909a0a9..19b39884c1ca3 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) {