test: replace proxyquire with Symbol options

Since proxyquire does not support native ECMAScript Modules, inject mocks using Symbol properties on options. The symbols are shared from a module which is not exported from the package to avoid expanding the API in a way that would allow external callers to inject their own versions. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kevinoid · Aug 5, 2021 · df5bbac · df5bbac
1 parent 9486a3d
commit df5bbac
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 33 deletions.
diff --git a/index.js b/index.js
@@ -9,6 +9,11 @@
 const fetchCiStatus = require('./lib/fetch-ci-status.js');
 const { resolveCommit } = require('./lib/git-utils.js');
 const { getProjectName } = require('./lib/github-utils.js');
+const {
+  fetchCiStatusMockSymbol,
+  getProjectNameMockSymbol,
+  resolveCommitMockSymbol,
+} = require('./lib/symbols.js');
 
 // Use same "severity" as hub(1) for determining state
 // https://github.com/github/hub/blob/v2.14.2/commands/ci_status.go#L60-L69
@@ -163,6 +168,9 @@ module.exports =
 async function hubCiStatus(
   rev = 'HEAD',
   {
+    [fetchCiStatusMockSymbol]: fetchCiStatusMock,
+    [getProjectNameMockSymbol]: getProjectNameMock,
+    [resolveCommitMockSymbol]: resolveCommitMock,
     gitOptions,
     octokit,
     octokitOptions,
@@ -176,9 +184,11 @@ async function hubCiStatus(
 ) {
   verbosity = Number(verbosity) || 0;
 
+  const getProjectNameOrMock = getProjectNameMock || getProjectName;
+  const resolveCommitOrMock = resolveCommitMock || resolveCommit;
   const [[owner, repo], ref] = await Promise.all([
-    getProjectName(gitOptions),
-    resolveCommit(rev, gitOptions),
+    getProjectNameOrMock(gitOptions),
+    resolveCommitOrMock(rev, gitOptions),
   ]);
   const statusOptions = {
     octokit,
@@ -189,8 +199,9 @@ async function hubCiStatus(
   if (verbosity > 1) {
     statusOptions.debug = (msg) => stderr.write(`DEBUG: ${msg}\n`);
   }
+  const fetchCiStatusOrMock = fetchCiStatusMock || fetchCiStatus;
   const [combinedStatus, checksList] =
-    await fetchCiStatus({ owner, repo, ref }, statusOptions);
+    await fetchCiStatusOrMock({ owner, repo, ref }, statusOptions);
 
   const statuses = [
     ...combinedStatus.statuses,

diff --git a/lib/fetch-ci-status.js b/lib/fetch-ci-status.js
@@ -13,6 +13,11 @@ const { promisify } = require('util');
 
 const getPackageJson = require('./get-package-json.js');
 const retryAsync = require('./retry-async.js');
+const {
+  HttpAgentMockSymbol,
+  HttpsAgentMockSymbol,
+  OctokitMockSymbol,
+} = require('./symbols.js');
 
 // TODO [engine:node@>=15]: import { setTimeout } from 'timers/promises';
 const setTimeoutP = promisify(timers.setTimeout);
@@ -22,6 +27,12 @@ async function fetchCiStatus(apiArgs, options = {}) {
   let agent;
   let { octokit } = options;
   if (octokit === undefined) {
+    const {
+      [HttpAgentMockSymbol]: HttpAgentMock,
+      [HttpsAgentMockSymbol]: HttpsAgentMock,
+      [OctokitMockSymbol]: OctokitMock,
+    } = options;
+
     const packageJson = await getPackageJson();
     const octokitOptions = {
       userAgent: `${packageJson.name}/${packageJson.version}`,
@@ -36,8 +47,8 @@ async function fetchCiStatus(apiArgs, options = {}) {
       const protocol =
         octokitOptions.baseUrl ? new URL(octokitOptions.baseUrl).protocol
           : 'https:';
-      const Agent = protocol === 'https:' ? HttpsAgent
-        : protocol === 'http:' ? HttpAgent
+      const Agent = protocol === 'https:' ? HttpsAgentMock || HttpsAgent
+        : protocol === 'http:' ? HttpAgentMock || HttpAgent
           : undefined;
       if (Agent) {
         agent = new Agent({ keepAlive: true });
@@ -48,7 +59,8 @@ async function fetchCiStatus(apiArgs, options = {}) {
       }
     }
 
-    octokit = new Octokit(octokitOptions);
+    const OctokitOrMock = OctokitMock || Octokit;
+    octokit = new OctokitOrMock(octokitOptions);
   }
 
   async function getStatus() {

diff --git a/lib/symbols.js b/lib/symbols.js
@@ -0,0 +1,43 @@
+/**
+ * @copyright Copyright 2021 Kevin Locke <kevin@kevinlocke.name>
+ * @license MIT
+ *
+ * Package-private symbols.
+ * This file is intentionally excluded from package.json#exports.
+ */
+
+/** Symbol of mock function used in place of http.Agent for testing.
+ *
+ * @private
+ */
+exports.HttpAgentMockSymbol = Symbol('HttpAgent');
+
+/** Symbol of mock function used in place of https.Agent for testing.
+ *
+ * @private
+ */
+exports.HttpsAgentMockSymbol = Symbol('HttpsAgent');
+
+/** Symbol of mock function used in place of Octokit for testing.
+ *
+ * @private
+ */
+exports.OctokitMockSymbol = Symbol('Octokit');
+
+/** Symbol of mock function used in place of fetchCiStatus for testing.
+ *
+ * @private
+ */
+exports.fetchCiStatusMockSymbol = Symbol('fetchCiStatus');
+
+/** Symbol of mock function used in place of getProjectName for testing.
+ *
+ * @private
+ */
+exports.getProjectNameMockSymbol = Symbol('getProjectName');
+
+/** Symbol of mock function used in place of resolveCommit for testing.
+ *
+ * @private
+ */
+exports.resolveCommitMockSymbol = Symbol('resolveCommit');
diff --git a/package.json b/package.json
@@ -74,7 +74,6 @@
     "jsdoc": "^3.6.0",
     "mocha": "^9.0.0",
     "nodecat": "^2.0.0",
-    "proxyquire": "^2.1.3",
     "rimraf": "^3.0.0",
     "sinon": "^11.0.0",
     "tmp-promise": "^3.0.2"

diff --git a/test/index.js b/test/index.js
@@ -7,26 +7,23 @@
 
 const ansiStyles = require('ansi-styles');
 const assert = require('assert');
-const proxyquire = require('proxyquire');
 const sinon = require('sinon');
 const { PassThrough } = require('stream');
 
+const hubCiStatus = require('..');
 const { makeCheckRuns, makeCombinedStatus } =
   require('../test-lib/api-responses.js');
+const {
+  fetchCiStatusMockSymbol,
+  getProjectNameMockSymbol,
+  resolveCommitMockSymbol,
+} = require('../lib/symbols.js');
 
 const { match } = sinon;
 
 const fetchCiStatus = sinon.stub();
 const getProjectName = sinon.stub();
 const resolveCommit = sinon.stub();
-const hubCiStatus = proxyquire(
-  '..',
-  {
-    './lib/fetch-ci-status.js': fetchCiStatus,
-    './lib/git-utils.js': { resolveCommit },
-    './lib/github-utils.js': { getProjectName },
-  },
-);
 
 const testOwner = 'owner';
 const testRepo = 'repo';
@@ -40,6 +37,10 @@ let testOptions;
 
 beforeEach(() => {
   testOptions = {
+    [fetchCiStatusMockSymbol]: fetchCiStatus,
+    [getProjectNameMockSymbol]: getProjectName,
+    [resolveCommitMockSymbol]: resolveCommit,
+
     stdout: new PassThrough({ encoding: 'utf8' }),
     stderr: new PassThrough({ encoding: 'utf8' }),
   };

diff --git a/test/lib/fetch-ci-status.js b/test/lib/fetch-ci-status.js
@@ -7,7 +7,6 @@
 
 const FakeTimers = require('@sinonjs/fake-timers');
 const assert = require('assert');
-const proxyquire = require('proxyquire');
 const sinon = require('sinon');
 const timers = require('timers');
 const { promisify } = require('util');
@@ -16,6 +15,11 @@ const fetchCiStatus = require('../../lib/fetch-ci-status.js');
 const getPackageJson = require('../../lib/get-package-json.js');
 const { makeCheckRuns, makeCombinedStatus } =
   require('../../test-lib/api-responses.js');
+const {
+  HttpAgentMockSymbol,
+  HttpsAgentMockSymbol,
+  OctokitMockSymbol,
+} = require('../../lib/symbols.js');
 
 // TODO [engine:node@>=15]: import { setImmediate } from 'timers/promises';
 const setImmediateP = promisify(timers.setImmediate);
@@ -468,15 +472,12 @@ describe('fetchCiStatus', () => {
     const httpsAgent = { destroy: sinon.stub() };
     const https = { Agent: sinon.stub().returns(httpsAgent) };
 
-    // eslint-disable-next-line no-shadow
-    const fetchCiStatus = proxyquire(
-      '../../lib/fetch-ci-status.js',
-      {
-        '@octokit/rest': { Octokit },
-        http,
-        https,
-      },
-    );
+    const mockOptions = Object.freeze({
+      [HttpAgentMockSymbol]: http.Agent,
+      [HttpsAgentMockSymbol]: https.Agent,
+      [OctokitMockSymbol]: Octokit,
+    });
+
     beforeEach(() => {
       Octokit.resetHistory();
       listForRef.resetHistory();
@@ -490,6 +491,7 @@ describe('fetchCiStatus', () => {
 
     it('does not construct Octokit with options.options', async () => {
       const options = {
+        ...mockOptions,
         octokit: {
           checks: { listForRef },
           repos: { getCombinedStatusForRef },
@@ -500,7 +502,7 @@ describe('fetchCiStatus', () => {
     });
 
     it('constructs Octokit with userAgent by default', async () => {
-      await fetchCiStatus(apiArgs);
+      await fetchCiStatus(apiArgs, mockOptions);
       const packageJson = await getPackageJson();
       sinon.assert.calledOnceWithExactly(Octokit, match({
         request: undefined,
@@ -510,23 +512,30 @@ describe('fetchCiStatus', () => {
     });
 
     it('passes octokitOptions to Octokit constructor', async () => {
-      const octokitOptions = {
-        foo: 'bar',
-        userAgent: 'testagent',
+      const options = {
+        ...mockOptions,
+        octokitOptions: {
+          foo: 'bar',
+          userAgent: 'testagent',
+        },
       };
-      await fetchCiStatus(apiArgs, { octokitOptions });
-      sinon.assert.calledOnceWithExactly(Octokit, match(octokitOptions));
+      await fetchCiStatus(apiArgs, options);
+      sinon.assert.calledOnceWithExactly(
+        Octokit,
+        match(options.octokitOptions),
+      );
       sinon.assert.calledWithNew(Octokit);
     });
 
     it('does not create Agent by default', async () => {
-      await fetchCiStatus(apiArgs);
+      await fetchCiStatus(apiArgs, mockOptions);
       sinon.assert.callCount(http.Agent, 0);
       sinon.assert.callCount(https.Agent, 0);
     });
 
     it('uses https.Agent with keep-alive for retries', async () => {
       const options = {
+        ...mockOptions,
         retry: timeOptions,
       };
       await fetchCiStatus(apiArgs, options);
@@ -547,6 +556,7 @@ describe('fetchCiStatus', () => {
 
     it('uses http.Agent with keep-alive for http baseUrl', async () => {
       const options = {
+        ...mockOptions,
         octokitOptions: {
           baseUrl: 'http://example.com',
           request: {},
@@ -571,6 +581,7 @@ describe('fetchCiStatus', () => {
 
     it('does not create Agent for unrecognized baseUrl', async () => {
       const options = {
+        ...mockOptions,
         octokitOptions: {
           baseUrl: 'foo://bar',
         },
@@ -583,6 +594,7 @@ describe('fetchCiStatus', () => {
 
     it('does not create Agent if null passed', async () => {
       const options = {
+        ...mockOptions,
         octokitOptions: {
           baseUrl: 'foo://bar',
           request: {