cherry-pick(#12300): chore: best-effort cleanup for output folders th…

…at are mounted (#12318) Fixes #12106
microsoft · Feb 23, 2022 · 0a1c1da · 0a1c1da
1 parent 2e41670
commit 0a1c1da
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 14 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/playwright-core/src/utils/utils.ts b/packages/playwright-core/src/utils/utils.ts
@@ -21,6 +21,7 @@ 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';
@@ -29,6 +30,8 @@ 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.
@@ -415,11 +418,32 @@ 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 }, error => {
-        fulfill(error ?? undefined);
+      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);
+        }
       });
     });
   }));

diff --git a/packages/playwright-test/package.json b/packages/playwright-test/package.json
@@ -61,7 +61,6 @@
     "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
@@ -15,7 +15,6 @@
  * limitations under the License.
  */
 
-import rimraf from 'rimraf';
 import * as fs from 'fs';
 import * as path from 'path';
 import { promisify } from 'util';
@@ -38,9 +37,8 @@ import { Minimatch } from 'minimatch';
 import { Config, FullConfig } from './types';
 import { WebServer } from './webServer';
 import { raceAgainstTimeout } from 'playwright-core/lib/utils/async';
-import { SigIntWatcher } from 'playwright-core/lib/utils/utils';
+import { removeFoldersOrDie, 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'];
@@ -352,7 +350,7 @@ export class Runner {
 
     // 12. Remove output directores.
     try {
-      await Promise.all(Array.from(outputDirs).map(outputDir => removeFolderAsync(outputDir)));
+      await removeFoldersOrDie(Array.from(outputDirs));
     } 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
@@ -14,8 +14,6 @@
  * 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';
@@ -27,10 +25,9 @@ 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;
@@ -308,7 +305,7 @@ export class WorkerRunner extends EventEmitter {
     const preserveOutput = this._loader.fullConfig().preserveOutput === 'always' ||
       (this._loader.fullConfig().preserveOutput === 'failures-only' && isFailure);
     if (!preserveOutput)
-      await removeFolderAsync(testInfo.outputDir).catch(e => {});
+      await removeFolders([testInfo.outputDir]);
   }
 
   private async _runTestWithBeforeHooks(test: TestCase, testInfo: TestInfoImpl) {