From d07e05fa7bb7a6b1e7b8f9bb2e169e72423b5b69 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 23 Feb 2022 15:10:27 -0700 Subject: [PATCH] Revert "cherry-pick(#12300): chore: best-effort cleanup for output folders that are mounted (#12318)" (#12322) 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 d1acd3d4d2..373d7eafb2 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 764eb0c5cd..7bc5c94c3f 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 edddbc04bf..4cffc31575 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 bd2d94b551..17dcd19a1f 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 ed6e3f1f3f..f421ade8b7 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) {