From f1488b6c3a8f4f856eaaf4869fdf742252b7f816 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Tue, 21 Mar 2023 11:39:15 +0100 Subject: [PATCH] chore: replace rimraf in tests with a helper with retries (#9888) --- test/src/headful.spec.ts | 7 ++++--- test/src/launcher.spec.ts | 43 ++++++++++++++++++++++----------------- test/src/mocha-utils.ts | 11 ++++++++-- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/test/src/headful.spec.ts b/test/src/headful.spec.ts index a22fba1561e..a27f5527494 100644 --- a/test/src/headful.spec.ts +++ b/test/src/headful.spec.ts @@ -23,9 +23,8 @@ import { PuppeteerLaunchOptions, PuppeteerNode, } from 'puppeteer-core/internal/node/PuppeteerNode.js'; -import rimraf from 'rimraf'; -import {getTestState} from './mocha-utils.js'; +import {getTestState, rmSync} from './mocha-utils.js'; const TMP_FOLDER = path.join(os.tmpdir(), 'pptr_tmp_folder-'); @@ -237,7 +236,9 @@ describe('headful tests', function () { }); await headlessBrowser.close(); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rimraf(userDataDir).catch(() => {}); + try { + rmSync(userDataDir); + } catch {} expect(cookie).toBe('foo=true'); }); // TODO: Support OOOPIF. @see https://github.com/puppeteer/puppeteer/issues/2548 diff --git a/test/src/launcher.spec.ts b/test/src/launcher.spec.ts index d88b2edb00c..b5446f10dff 100644 --- a/test/src/launcher.spec.ts +++ b/test/src/launcher.spec.ts @@ -23,16 +23,11 @@ import {Protocol} from 'devtools-protocol'; import expect from 'expect'; import {BrowserFetcher, TimeoutError} from 'puppeteer'; import {Page} from 'puppeteer-core/internal/api/Page.js'; -import rimraf from 'rimraf'; import sinon from 'sinon'; -import {getTestState, itOnlyRegularInstall} from './mocha-utils.js'; +import {getTestState, itOnlyRegularInstall, rmSync} from './mocha-utils.js'; import utils from './utils.js'; -const rmAsync = (filename: string) => { - return rimraf(filename); -}; - const TMP_FOLDER = path.join(os.tmpdir(), 'pptr_tmp_folder-'); const FIREFOX_TIMEOUT = 30 * 1000; @@ -77,12 +72,10 @@ describe('Launcher specs', function () { expect((await stat(revisionInfo.executablePath)).mode & 0o777).toBe( expectedPermissions ); - expect(await browserFetcher.localRevisions()).toEqual([ - expectedRevision, - ]); + expect(browserFetcher.localRevisions()).toEqual([expectedRevision]); await browserFetcher.remove(expectedRevision); - expect(await browserFetcher.localRevisions()).toEqual([]); - await rmAsync(downloadsFolder); + expect(browserFetcher.localRevisions()).toEqual([]); + rmSync(downloadsFolder); }); it('should download and extract firefox linux binary', async () => { const {server} = getTestState(); @@ -127,7 +120,7 @@ describe('Launcher specs', function () { ]); await browserFetcher.remove(expectedVersion); expect(await browserFetcher.localRevisions()).toEqual([]); - await rmAsync(downloadsFolder); + rmSync(downloadsFolder); }); }); @@ -248,7 +241,9 @@ describe('Launcher specs', function () { await browser.close(); expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + try { + rmSync(userDataDir); + } catch {} }); it('tmp profile should be cleaned up', async () => { const {defaultBrowserOptions, puppeteer} = getTestState(); @@ -301,7 +296,9 @@ describe('Launcher specs', function () { expect(await readFile(prefsJSPath, 'utf8')).toBe(prefsJSContent); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + try { + rmSync(userDataDir); + } catch {} }); it('userDataDir argument', async () => { const {isChrome, puppeteer, defaultBrowserOptions} = getTestState(); @@ -325,13 +322,15 @@ describe('Launcher specs', function () { await browser.close(); expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + try { + rmSync(userDataDir); + } catch {} }); it('userDataDir argument with non-existent dir', async () => { const {isChrome, puppeteer, defaultBrowserOptions} = getTestState(); const userDataDir = await mkdtemp(TMP_FOLDER); - await rmAsync(userDataDir); + rmSync(userDataDir); const options = Object.assign({}, defaultBrowserOptions); if (isChrome) { options.args = [ @@ -350,7 +349,9 @@ describe('Launcher specs', function () { await browser.close(); expect(fs.readdirSync(userDataDir).length).toBeGreaterThan(0); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + try { + rmSync(userDataDir); + } catch {} }); it('userDataDir option should restore state', async () => { const {server, puppeteer, defaultBrowserOptions} = getTestState(); @@ -375,7 +376,9 @@ describe('Launcher specs', function () { ).toBe('hello'); await browser2.close(); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + try { + rmSync(userDataDir); + } catch {} }); it('userDataDir option should restore cookies', async () => { const {server, puppeteer, defaultBrowserOptions} = getTestState(); @@ -401,7 +404,9 @@ describe('Launcher specs', function () { ).toBe('doSomethingOnlyOnce=true'); await browser2.close(); // This might throw. See https://github.com/puppeteer/puppeteer/issues/2778 - await rmAsync(userDataDir).catch(() => {}); + try { + rmSync(userDataDir); + } catch {} }); it('should return the default arguments', async () => { const {isChrome, isFirefox, puppeteer} = getTestState(); diff --git a/test/src/mocha-utils.ts b/test/src/mocha-utils.ts index 18130d17df7..d8a4b6f40c0 100644 --- a/test/src/mocha-utils.ts +++ b/test/src/mocha-utils.ts @@ -34,7 +34,6 @@ import { PuppeteerNode, } from 'puppeteer-core/internal/node/PuppeteerNode.js'; import {isErrorLike} from 'puppeteer-core/internal/util/ErrorLike.js'; -import rimraf from 'rimraf'; import sinon from 'sinon'; import {extendExpectWithToBeGolden} from './utils.js'; @@ -128,7 +127,7 @@ const setupGoldenAssertions = (): void => { const GOLDEN_DIR = path.join(__dirname, `../golden-${suffix}`); const OUTPUT_DIR = path.join(__dirname, `../output-${suffix}`); if (fs.existsSync(OUTPUT_DIR)) { - rimraf.sync(OUTPUT_DIR); + rmSync(OUTPUT_DIR); } extendExpectWithToBeGolden(GOLDEN_DIR, OUTPUT_DIR); }; @@ -332,3 +331,11 @@ export const createTimeout = ( }, n); }); }; + +export function rmSync(target: string): void { + fs.rmSync(target, { + force: true, + recursive: true, + maxRetries: 5, + }); +}