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`.

diff --git a/src/command.js b/src/command.js
index 1035eb386d73142ffc4df67ff53a930038045a34..adea1c9c5b887dcb84eb2d830768397845b6cdc5 100644 (file)
--- 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 eaed3f776cdda13a62836923393f74f4f7c4bc8f..bb2de1913e4745d26c0ccab0184157cc69f43018 100644 (file)
--- 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 (file)
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 e48e4363a99117893848782f5fbc0cf952f549a3..679f5cda48c81ac09f96a021f5ce73432ec15c4a 100644 (file)
--- 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 rejection = rejects(encrypt({ inFile: envFile }), { name: 'InputError', message: 'Unable to get passkey.' });
+    const { promise: opened, resolve } = /** @type {PromiseWithResolvers<void>}*/(Promise.withResolvers());
+    /** @type {(url: string) => Promise<void>} */
+    const onListening = async url => {
+      await page.goto(url);
+      resolve();
+    }
+
+    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 rejection = rejects(decrypt({ inFile: envFile }), { name: 'InputError', message: 'Unable to get passkey.' });
+    const { promise: opened, resolve } = /** @type {PromiseWithResolvers<void>}*/(Promise.withResolvers());
+    /** @type {(url: string) => Promise<void>} */
+    const onListening = async url => {
+      await page.goto(url);
+      resolve();
+    }
+
+    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 c7dd4ca979bf6fabd72a9cef74412d0acf00b711..c88d088233703856629374027b2cb6bfc9ad6f6c 100644 (file)
--- 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<void>} Promise which resolves when the Playwright browser has opened the page.
- */
-
-export function mockOpen(t, page) {
-  const { promise, resolve } = /** @type {PromiseWithResolvers<void>}*/(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.