From 5a63df5a529453f57130e60c70eaed3f9d7ef708 Mon Sep 17 00:00:00 2001 From: Ardis Lu Date: Sun, 6 Sep 2026 22:55:32 -0700 Subject: [PATCH] Refactor global `BROWSER` singleton into dependency injection DI is required to enable concurrency in tests and should generally reduce flakiness. To complete the refactor and enable concurrency, the other global singleton `SERVER` must also be refactored, and a different approach to `assertConsole` must be used instead of mocking the global `console`. --- src/command.js | 4 +-- src/getPrf.js | 16 +++++------ test/decrypt.test.js | 30 --------------------- test/index.test.js | 63 +++++++++++++++++++++----------------------- test/setup.js | 22 ++-------------- 5 files changed, 40 insertions(+), 95 deletions(-) delete mode 100644 test/decrypt.test.js diff --git a/src/command.js b/src/command.js index 1035eb3..adea1c9 100644 --- a/src/command.js +++ b/src/command.js @@ -37,7 +37,7 @@ export async function encrypt(options = {}) { inFile = '.env', outFile = inFile, alreadyEncryptedValue = 'ignore', - getPrfOptions = { autoOpen: true, port: undefined }, + getPrfOptions = { port: undefined }, logger = {} } = options; @@ -118,7 +118,7 @@ export async function decrypt(options = {}, { args = [] } = {}) { outFile = inFile, injectInProcess = false, notEncryptedValue = 'ignore', - getPrfOptions = { autoOpen: true, port: undefined }, + getPrfOptions = { port: undefined }, logger = {} } = options; diff --git a/src/getPrf.js b/src/getPrf.js index eaed3f7..bb2de19 100644 --- a/src/getPrf.js +++ b/src/getPrf.js @@ -14,12 +14,6 @@ async function getHTML() { return await readFile(new URL('./getPrf.html', import.meta.url), { encoding: 'utf8' }); } -/** - * Wrapper around browser operations. This wrapper object is required to mock the methods for - * testing purposes. - */ -export const BROWSER = { open }; - /** * Wrapper around server operations. This wrapper object is required to mock the methods for * testing purposes. @@ -34,8 +28,10 @@ export const WINDOW_EXPIRATION_DURATION = 2 * 60 * 1000; /** * @typedef {Object} GetPrfOptions - * @property {boolean} [autoOpen] Automatically open a web browser to the passkey page. The default - * value is `true`. + * @property {((url: string) => unknown)|null} [onListening] Function which will be called after the server is ready. + * The function is passed a URL on `localhost` to complete the passkey flow. The default value is + * {@link https://github.com/sindresorhus/open|`open`}, which will open the default web browser to the passkey page. + * To disable auto-opening, set this value to `null`. * @property {number} [port] IP port number to use for the local server hosting the passkey page. By * default, defer to the operating system to assign a number. * @property {AbortSignal} [signal] `AbortSignal` which may be used to cancel this passkey flow. @@ -63,7 +59,7 @@ export const WINDOW_EXPIRATION_DURATION = 2 * 60 * 1000; */ export async function getPrf(options = {}) { const { - autoOpen = true, + onListening = open, port = undefined, signal = undefined } = options; @@ -200,7 +196,7 @@ export async function getPrf(options = {}) { const { port } = /** @type {AddressInfo} */(server.address()); const url = `http://localhost:${port}?challenge=${challenge}&exp=${timeoutExp}`; urlResolve(url); - if (autoOpen) { BROWSER.open(url); } + if (onListening !== null) { onListening(url); } }); return { diff --git a/test/decrypt.test.js b/test/decrypt.test.js deleted file mode 100644 index 6d91a76..0000000 --- a/test/decrypt.test.js +++ /dev/null @@ -1,30 +0,0 @@ -import { suite, test } from 'node:test'; -import { deepStrictEqual } from 'node:assert/strict'; -import { chdir, cwd } from 'node:process'; - -import { encrypt } from '#src/index.js'; -import { setupPlaywright, mockOpen, setupFolder, fileEqual, fileNotEqual } from '#test/setup.js'; - -suite('import @ardislu/envpass/decrypt', () => { - test('it works', async (t) => { - const env = 'secret=some_secret_value'; - const { page } = await setupPlaywright(t); - const { folder, envFile } = await setupFolder(t, env); - mockOpen(t, page); - - const oldDir = cwd(); - chdir(folder); - - // Verifies .env is encrypted - await fileEqual(envFile, env); - await encrypt(); - await fileNotEqual(envFile, env); - - deepStrictEqual(process.env.secret, undefined); - await import('#src/decrypt.js'); - deepStrictEqual(process.env.secret, 'some_secret_value'); // Successfully decrypted and injected in process - await fileNotEqual(envFile, env); // .env is NOT decrypted - - chdir(oldDir); // Release lock on temp folder to allow teardown - }); -}); \ No newline at end of file diff --git a/test/index.test.js b/test/index.test.js index e48e436..679f5cd 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -3,8 +3,8 @@ import { deepStrictEqual, rejects } from 'node:assert'; import childProcess from 'node:child_process'; import { encrypt, decrypt } from '#src/index.js'; -import { BROWSER, WINDOW_EXPIRATION_DURATION } from '#src/getPrf.js'; -import { setupPlaywright, setupEnv, mockOpen, assertConsole, MockLogger, fileEqual, fileNotEqual } from '#test/setup.js'; +import { WINDOW_EXPIRATION_DURATION } from '#src/getPrf.js'; +import { setupPlaywright, setupEnv, assertConsole, MockLogger, fileEqual, fileNotEqual } from '#test/setup.js'; /** Standard value of the .env file used for most setups. */ const STD_ENV = 'A=123\nB=456\nC=789\n'; @@ -17,42 +17,39 @@ suite('e2e', () => { test('encrypt and decrypt .env', async (t) => { const { page } = await setupPlaywright(t); const envFile = await setupEnv(t, STD_ENV); - mockOpen(t, page); assertConsole(t, { debug: 0, info: 0, warn: 0, error: 0 }); await fileEqual(envFile, STD_ENV); - await encrypt({ inFile: envFile }); + await encrypt({ inFile: envFile, getPrfOptions: { onListening: url => page.goto(url) } }); await fileNotEqual(envFile, STD_ENV); - await decrypt({ inFile: envFile }); + await decrypt({ inFile: envFile, getPrfOptions: { onListening: url => page.goto(url) } }); await fileEqual(envFile, STD_ENV); }); test('console logger works', async (t) => { const { page } = await setupPlaywright(t); const envFile = await setupEnv(t, STD_ENV); - mockOpen(t, page); assertConsole(t, { debug: 24, info: 6, warn: 0, error: 0 }); await fileEqual(envFile, STD_ENV); - await encrypt({ inFile: envFile, logger: console }); - await encrypt({ inFile: envFile, logger: console, alreadyEncryptedValue: 'log' }); + await encrypt({ inFile: envFile, logger: console, getPrfOptions: { onListening: url => page.goto(url) } }); + await encrypt({ inFile: envFile, logger: console, alreadyEncryptedValue: 'log', getPrfOptions: { onListening: url => page.goto(url) } }); await fileNotEqual(envFile, STD_ENV); - await decrypt({ inFile: envFile, logger: console }); - await decrypt({ inFile: envFile, logger: console, notEncryptedValue: 'log' }); + await decrypt({ inFile: envFile, logger: console, getPrfOptions: { onListening: url => page.goto(url) } }); + await decrypt({ inFile: envFile, logger: console, notEncryptedValue: 'log', getPrfOptions: { onListening: url => page.goto(url) } }); await fileEqual(envFile, STD_ENV); }); test('alternate logger works', async (t) => { const { page } = await setupPlaywright(t); const envFile = await setupEnv(t, STD_ENV); - mockOpen(t, page); assertConsole(t, { debug: 0, info: 0, warn: 0, error: 0 }); const logger = new MockLogger(); await fileEqual(envFile, STD_ENV); - await encrypt({ inFile: envFile, logger }); - await encrypt({ inFile: envFile, logger, alreadyEncryptedValue: 'log' }); + await encrypt({ inFile: envFile, logger, getPrfOptions: { onListening: url => page.goto(url) } }); + await encrypt({ inFile: envFile, logger, alreadyEncryptedValue: 'log', getPrfOptions: { onListening: url => page.goto(url) } }); await fileNotEqual(envFile, STD_ENV); - await decrypt({ inFile: envFile, logger }); - await decrypt({ inFile: envFile, logger, notEncryptedValue: 'log' }); + await decrypt({ inFile: envFile, logger, getPrfOptions: { onListening: url => page.goto(url) } }); + await decrypt({ inFile: envFile, logger, notEncryptedValue: 'log', getPrfOptions: { onListening: url => page.goto(url) } }); await fileEqual(envFile, STD_ENV); logger.assertCounts({ debug: 24, info: 6, warn: 0, error: 0 }); }); @@ -62,40 +59,35 @@ suite('encrypt', () => { test('does nothing to empty .env file', async (t) => { const { page } = await setupPlaywright(t); const envFile = await setupEnv(t, ''); - mockOpen(t, page); assertConsole(t, { debug: 0, info: 0, warn: 0, error: 0 }); await fileEqual(envFile, ''); - await encrypt({ inFile: envFile }); + await encrypt({ inFile: envFile, getPrfOptions: { onListening: url => page.goto(url) } }); await fileEqual(envFile, ''); }); test('does nothing to empty .env file (with logs)', async (t) => { const { page } = await setupPlaywright(t); const envFile = await setupEnv(t, ''); - mockOpen(t, page); assertConsole(t, { debug: 1, info: 1, warn: 0, error: 0 }); await fileEqual(envFile, ''); - await encrypt({ inFile: envFile, logger: console }); + await encrypt({ inFile: envFile, logger: console, getPrfOptions: { onListening: url => page.goto(url) } }); await fileEqual(envFile, ''); }); test('does nothing when all variables are already encrypted', async (t) => { - const openMock = t.mock.method(BROWSER, 'open'); const envFile = await setupEnv(t, ENCRYPTED_ENV); assertConsole(t, { debug: 0, info: 0, warn: 0, error: 0 }); await encrypt({ inFile: envFile }); await fileEqual(envFile, ENCRYPTED_ENV); - deepStrictEqual(openMock.mock.calls.length, 0); }); test("encrypts again when alreadyEncryptedValue: 'encrypt'", async (t) => { const { page } = await setupPlaywright(t); const envFile = await setupEnv(t, ENCRYPTED_ENV); - mockOpen(t, page); assertConsole(t, { debug: 0, info: 0, warn: 0, error: 0 }); - await encrypt({ inFile: envFile, alreadyEncryptedValue: 'encrypt' }); + await encrypt({ inFile: envFile, alreadyEncryptedValue: 'encrypt', getPrfOptions: { onListening: url => page.goto(url) } }); await fileNotEqual(envFile, ENCRYPTED_ENV); }); @@ -109,11 +101,16 @@ suite('encrypt', () => { t.mock.timers.enable({ apis: ['setTimeout'] }); const { page, setAutomaticSignIn } = await setupPlaywright(t); await setAutomaticSignIn(false); - const opened = mockOpen(t, page); const envFile = await setupEnv(t, STD_ENV); assertConsole(t, { debug: 0, info: 0, warn: 0, error: 0 }); + const { promise: opened, resolve } = /** @type {PromiseWithResolvers}*/(Promise.withResolvers()); + /** @type {(url: string) => Promise} */ + const onListening = async url => { + await page.goto(url); + resolve(); + } - const rejection = rejects(encrypt({ inFile: envFile }), { name: 'InputError', message: 'Unable to get passkey.' }); + const rejection = rejects(encrypt({ inFile: envFile, getPrfOptions: { onListening } }), { name: 'InputError', message: 'Unable to get passkey.' }); await opened; t.mock.timers.tick(WINDOW_EXPIRATION_DURATION); await rejection; @@ -122,9 +119,8 @@ suite('encrypt', () => { suite('decrypt', () => { test('does nothing to empty .env file', async (t) => { - const { page } = await setupPlaywright(t); + await setupPlaywright(t); const envFile = await setupEnv(t, ''); - mockOpen(t, page); assertConsole(t, { debug: 0, info: 0, warn: 0, error: 0 }); await fileEqual(envFile, ''); @@ -132,9 +128,7 @@ suite('decrypt', () => { await fileEqual(envFile, ''); }); test('does nothing to empty .env file (with logs)', async (t) => { - const { page } = await setupPlaywright(t); const envFile = await setupEnv(t, ''); - mockOpen(t, page); assertConsole(t, { debug: 1, info: 1, warn: 0, error: 0 }); await fileEqual(envFile, ''); @@ -142,14 +136,12 @@ suite('decrypt', () => { await fileEqual(envFile, ''); }); test('does nothing when all variables are already decrypted', async (t) => { - const openMock = t.mock.method(BROWSER, 'open'); const envFile = await setupEnv(t, STD_ENV); assertConsole(t, { debug: 0, info: 0, warn: 0, error: 0 }); await decrypt({ inFile: envFile }); await fileEqual(envFile, STD_ENV); - deepStrictEqual(openMock.mock.calls.length, 0); }); test("throws error when notEncryptedValue='error'", async (t) => { const envFile = await setupEnv(t, STD_ENV); @@ -161,12 +153,17 @@ suite('decrypt', () => { t.mock.timers.enable({ apis: ['setTimeout'] }); const { page, setAutomaticSignIn } = await setupPlaywright(t); await setAutomaticSignIn(false); - const opened = mockOpen(t, page); const env = 'A=envpass:v1:AAA\nB=envpass:v1:BBB\nC=envpass:v1:CCC\n'; const envFile = await setupEnv(t, env); assertConsole(t, { debug: 0, info: 0, warn: 0, error: 0 }); + const { promise: opened, resolve } = /** @type {PromiseWithResolvers}*/(Promise.withResolvers()); + /** @type {(url: string) => Promise} */ + const onListening = async url => { + await page.goto(url); + resolve(); + } - const rejection = rejects(decrypt({ inFile: envFile }), { name: 'InputError', message: 'Unable to get passkey.' }); + const rejection = rejects(decrypt({ inFile: envFile, getPrfOptions: { onListening } }), { name: 'InputError', message: 'Unable to get passkey.' }); await opened; t.mock.timers.tick(WINDOW_EXPIRATION_DURATION); await rejection; diff --git a/test/setup.js b/test/setup.js index c7dd4ca..c88d088 100644 --- a/test/setup.js +++ b/test/setup.js @@ -7,7 +7,7 @@ import { deepStrictEqual, ok } from 'node:assert/strict'; import { chromium } from 'playwright-core'; /** @import { Page } from 'playwright-core'; */ -import { getPrf, BROWSER } from '#src/getPrf.js'; +import { getPrf } from '#src/getPrf.js'; /** @import { GetPrfResult } from '#src/getPrf.js'; */ /** @@ -66,7 +66,7 @@ export async function setupPlaywright(t, options = {}) { */ export async function setupServer(t) { const controller = new AbortController(); - const { url, prf } = await getPrf({ autoOpen: false, signal: controller.signal }); + const { url, prf } = await getPrf({ onListening: null, signal: controller.signal }); t.after(() => controller.abort()); return { controller, url, prf }; } @@ -111,24 +111,6 @@ export async function setupFolder(t, env = '') { return { folder, envFile }; } -/** - * Intercept the `open` function to open a Playwright browser to the page instead of the user's own browser. - * @param {TestContext} t Test context to mock the `open` function for. - * @param {Page} page Playwright `Page` to use to navigate to the URL instead of the user's browser. - * @returns {Promise} Promise which resolves when the Playwright browser has opened the page. - */ - -export function mockOpen(t, page) { - const { promise, resolve } = /** @type {PromiseWithResolvers}*/(Promise.withResolvers()); - /** @param {string} url */ - const interceptor = async url => { - await page.goto(url); - resolve(); - } - t.mock.method(BROWSER, 'open', interceptor); - return promise; -} - /** * @typedef AssertConsoleCounts The total number of times `console.debug`, `console.info`, `console.warn`, and * `console.error` are expected to be called. -- 2.47.3