From b497b818f92991c1e9d92717cc6d087994809174 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Tue, 4 May 2021 13:57:11 +0200 Subject: [PATCH 1/4] test: add test for relative downloadsPath folder --- src/server/artifact.ts | 3 ++- tests/config/default.config.ts | 2 +- tests/downloads-path.spec.ts | 15 +++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/server/artifact.ts b/src/server/artifact.ts index d5fe84bbc0428..c1908a874ff23 100644 --- a/src/server/artifact.ts +++ b/src/server/artifact.ts @@ -15,6 +15,7 @@ */ import fs from 'fs'; +import path from 'path'; import * as util from 'util'; import { SdkObject } from './instrumentation'; @@ -32,7 +33,7 @@ export class Artifact extends SdkObject { constructor(parent: SdkObject, localPath: string, unaccessibleErrorMessage?: string) { super(parent, 'artifact'); - this._localPath = localPath; + this._localPath = path.isAbsolute(localPath) ? localPath : path.join(process.cwd(), localPath); this._unaccessibleErrorMessage = unaccessibleErrorMessage; this._finishedCallback = () => {}; this._finishedPromise = new Promise(f => this._finishedCallback = f); diff --git a/tests/config/default.config.ts b/tests/config/default.config.ts index 056c86e985176..07f59f926acde 100644 --- a/tests/config/default.config.ts +++ b/tests/config/default.config.ts @@ -29,7 +29,7 @@ const config: folio.Config = { }; if (process.env.CI) { config.workers = 1; - config.forbidOnly = true; + // config.forbidOnly = true; config.retries = 3; } folio.setConfig(config); diff --git a/tests/downloads-path.spec.ts b/tests/downloads-path.spec.ts index 11ff43c2d2ca8..98214f2058ed4 100644 --- a/tests/downloads-path.spec.ts +++ b/tests/downloads-path.spec.ts @@ -16,6 +16,7 @@ import { playwrightTest as it, expect } from './config/browserTest'; import fs from 'fs'; +import path from 'path'; it.describe('downloads path', () => { it.beforeEach(async ({server}) => { @@ -71,6 +72,20 @@ it.describe('downloads path', () => { await downloadsBrowser.close(); }); + it.only('should report downloads in downloadsPath folder with a relative path', async ({browserType, browserOptions, server}, testInfo) => { + const downloadsBrowser = await browserType.launch({ ...browserOptions, downloadsPath: path.relative(process.cwd(), testInfo.outputPath('')) }); + const page = await downloadsBrowser.newPage({ acceptDownloads: true }); + await page.setContent(`download`); + const [ download ] = await Promise.all([ + page.waitForEvent('download'), + page.click('a') + ]); + const downloadPath = await download.path(); + expect(downloadPath.startsWith(testInfo.outputPath(''))).toBeTruthy(); + await page.close(); + await downloadsBrowser.close(); + }); + it('should accept downloads in persistent context', async ({launchPersistent, server}, testInfo) => { const { context, page } = await launchPersistent({ acceptDownloads: true, downloadsPath: testInfo.outputPath('') }); await page.setContent(`download`); From 3feec39466fa22dca444ea931369aae6ae79c401 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Tue, 4 May 2021 14:05:31 +0200 Subject: [PATCH 2/4] fix: path normalisation to an absolute path --- src/server/browserType.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/server/browserType.ts b/src/server/browserType.ts index 818339aa0fcc7..1c0bf72dfcf27 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -142,15 +142,14 @@ export abstract class BrowserType extends SdkObject { const tempDirectories = []; const ensurePath = async (tmpPrefix: string, pathFromOptions?: string) => { - let dir; if (pathFromOptions) { - dir = pathFromOptions; - await mkdirAsync(pathFromOptions, { recursive: true }); - } else { - dir = await mkdtempAsync(tmpPrefix); - tempDirectories.push(dir); + const absoltePathFromOptions = path.isAbsolute(pathFromOptions) ? pathFromOptions : path.join(process.cwd(), pathFromOptions); + await mkdirAsync(absoltePathFromOptions, { recursive: true }); + return absoltePathFromOptions; } - return dir; + const tmpDir = await mkdtempAsync(tmpPrefix); + tempDirectories.push(tmpDir); + return tmpDir; }; // TODO: add downloadsPath to newContext(). const downloadsPath = await ensurePath(DOWNLOADS_FOLDER, options.downloadsPath); From 4cfcc6c6a20c728006771c1b09198f2fdd4d6079 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Tue, 4 May 2021 14:10:41 +0200 Subject: [PATCH 3/4] remove forbidOnly hack --- tests/config/default.config.ts | 2 +- tests/downloads-path.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/config/default.config.ts b/tests/config/default.config.ts index 07f59f926acde..056c86e985176 100644 --- a/tests/config/default.config.ts +++ b/tests/config/default.config.ts @@ -29,7 +29,7 @@ const config: folio.Config = { }; if (process.env.CI) { config.workers = 1; - // config.forbidOnly = true; + config.forbidOnly = true; config.retries = 3; } folio.setConfig(config); diff --git a/tests/downloads-path.spec.ts b/tests/downloads-path.spec.ts index 98214f2058ed4..a5858da1723d0 100644 --- a/tests/downloads-path.spec.ts +++ b/tests/downloads-path.spec.ts @@ -72,7 +72,7 @@ it.describe('downloads path', () => { await downloadsBrowser.close(); }); - it.only('should report downloads in downloadsPath folder with a relative path', async ({browserType, browserOptions, server}, testInfo) => { + it('should report downloads in downloadsPath folder with a relative path', async ({browserType, browserOptions, server}, testInfo) => { const downloadsBrowser = await browserType.launch({ ...browserOptions, downloadsPath: path.relative(process.cwd(), testInfo.outputPath('')) }); const page = await downloadsBrowser.newPage({ acceptDownloads: true }); await page.setContent(`download`); From c954f76e814f71c29cf20413e844d15178a00b3b Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Wed, 5 May 2021 09:52:13 +0200 Subject: [PATCH 4/4] doing it now in validateLaunchOptions --- src/server/artifact.ts | 3 +-- src/server/browserType.ts | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/server/artifact.ts b/src/server/artifact.ts index c1908a874ff23..d5fe84bbc0428 100644 --- a/src/server/artifact.ts +++ b/src/server/artifact.ts @@ -15,7 +15,6 @@ */ import fs from 'fs'; -import path from 'path'; import * as util from 'util'; import { SdkObject } from './instrumentation'; @@ -33,7 +32,7 @@ export class Artifact extends SdkObject { constructor(parent: SdkObject, localPath: string, unaccessibleErrorMessage?: string) { super(parent, 'artifact'); - this._localPath = path.isAbsolute(localPath) ? localPath : path.join(process.cwd(), localPath); + this._localPath = localPath; this._unaccessibleErrorMessage = unaccessibleErrorMessage; this._finishedCallback = () => {}; this._finishedPromise = new Promise(f => this._finishedCallback = f); diff --git a/src/server/browserType.ts b/src/server/browserType.ts index 1c0bf72dfcf27..575e9ee25ddd7 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -142,14 +142,15 @@ export abstract class BrowserType extends SdkObject { const tempDirectories = []; const ensurePath = async (tmpPrefix: string, pathFromOptions?: string) => { + let dir; if (pathFromOptions) { - const absoltePathFromOptions = path.isAbsolute(pathFromOptions) ? pathFromOptions : path.join(process.cwd(), pathFromOptions); - await mkdirAsync(absoltePathFromOptions, { recursive: true }); - return absoltePathFromOptions; + dir = pathFromOptions; + await mkdirAsync(pathFromOptions, { recursive: true }); + } else { + dir = await mkdtempAsync(tmpPrefix); + tempDirectories.push(dir); } - const tmpDir = await mkdtempAsync(tmpPrefix); - tempDirectories.push(tmpDir); - return tmpDir; + return dir; }; // TODO: add downloadsPath to newContext(). const downloadsPath = await ensurePath(DOWNLOADS_FOLDER, options.downloadsPath); @@ -270,8 +271,10 @@ function copyTestHooks(from: object, to: object) { function validateLaunchOptions(options: Options): Options { const { devtools = false } = options; - let { headless = !devtools } = options; + let { headless = !devtools, downloadsPath } = options; if (debugMode()) headless = false; - return { ...options, devtools, headless }; + if (downloadsPath && !path.isAbsolute(downloadsPath)) + downloadsPath = path.join(process.cwd(), downloadsPath); + return { ...options, devtools, headless, downloadsPath }; }