Skip to content

Commit

Permalink
Revert "cherry-pick(#12300): chore: best-effort cleanup for output fo…
Browse files Browse the repository at this point in the history
…lders that are mounted (#12318)" (#12322)

This reverts commit 0a1c1da.

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
  • Loading branch information
aslushnikov authored Feb 23, 2022
1 parent 0a1c1da commit d07e05f
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 30 deletions.
2 changes: 2 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 2 additions & 26 deletions packages/playwright-core/src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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.
Expand Down Expand Up @@ -418,32 +415,11 @@ export function createGuid(): string {
return crypto.randomBytes(16).toString('hex');
}

export async function removeFoldersOrDie(dirs: string[]): Promise<void> {
const errors = await removeFolders(dirs);
for (const error of errors) {
if (error)
throw error;
}
}

export async function removeFolders(dirs: string[]): Promise<Array<Error|null|undefined>> {
return await Promise.all(dirs.map((dir: string) => {
return new Promise<Error|null|undefined>(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);
});
});
}));
Expand Down
1 change: 1 addition & 0 deletions packages/playwright-test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 4 additions & 2 deletions packages/playwright-test/src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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'];
Expand Down Expand Up @@ -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' };
Expand Down
7 changes: 5 additions & 2 deletions packages/playwright-test/src/workerRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit d07e05f

Please sign in to comment.