fix: aggregate verify conditions error (#33)

* chore: bump semantic-release to 16.0.1

* fix: include semantic-release in peer deps

* wip: add e2e

* wip: run e2e

* wip: remove e2e yarn lock

* fix: use aggregate error in verify conditions

* test: prepare

* test: publish

* test: verify conditions

.circleci/config.yml

@@ -55,6 +55,7 @@ jobs:
         steps:
             - prep
             - run: yarn test-coverage
+            - run: cd e2e && yarn test-e2e
             - persist_to_workspace:
                 root: ~/repo
                 paths:

e2e/.gitignore

@@ -0,0 +1 @@
+yarn.lock

e2e/package.json

@@ -0,0 +1,22 @@
+{
+  "dependencies": {
+    "web-ext": "^4.0.0",
+    "semantic-release": "^16.0.0"
+  },
+  "scripts": {
+    "test-e2e": "../node_modules/.bin/semantic-release"
+  },
+  "release": {
+    "dryRun": true,
+    "plugins": [
+      [
+        "../src/index.js",
+        {
+          "artifactsDir": "./",
+          "extensionId": "{01234567-abcd-6789-cdef-0123456789ef}",
+          "targetXpi": "extension.xpi"
+        }
+      ]
+    ]
+  }
+}

package.json

@@ -10,12 +10,17 @@
   "dependencies": {
     "web-ext": "^4.0.0"
   },
+  "peerDependencies": {
+    "aggregate-error": "^3.0.0",
+    "semantic-release": "^16.0.0"
+  },
   "devDependencies": {
     "@babel/core": "^7.6.0",
     "@babel/preset-env": "^7.6.0",
     "@tophat/commitizen-adapter": "^0.0.11",
     "@tophat/commitlint-config": "^0.1.2",
     "@tophat/eslint-config": "^0.4.0",
+    "aggregate-error": "^3.0.1",
     "all-contributors-cli": "^6.9.1",
     "babel-eslint": "^10.0.2",
     "codecov": "^3.5.0",
@@ -28,9 +33,11 @@
     "eslint-plugin-prettier": "^3.1.0",
     "husky": "^4.0.1",
     "jest": "^24.9.0",
+    "jest-mock-props": "^1.7.2",
     "lint-staged": "^9.2.5",
+    "memfs": "^3.0.3",
     "prettier": "^1.18.2",
-    "semantic-release": "^15.13.18",
+    "semantic-release": "^16.0.1",
     "yarn-deduplicate": "^1.1.1"
   },
   "scripts": {
@@ -71,6 +78,11 @@
       "pre-commit": "lint-staged"
     }
   },
+  "jest": {
+    "setupFilesAfterEnv": [
+      "./tests/setup.js"
+    ]
+  },
   "release": {
     "dryRun": false,
     "plugins": [

src/prepare.js

@@ -13,30 +13,26 @@ const prepare = (options, { nextRelease, logger }) => {
     try {
         manifest = fs.readFileSync(normalizedManifestPath)
     } catch (e) {
-        throw new Error('Unable to read manifest.json file from dist folder', e)
+        throw new Error('Unable to read manifest.json file from dist folder')
     }
 
     try {
-        const jsonManifest = JSON.parse(manifest)
+        const jsonManifest = JSON.parse(manifest.toString())
         jsonManifest.version = version
         manifest = JSON.stringify(jsonManifest, null, 2)
     } catch (e) {
-        throw new Error(
-            'Failed to parse manifest.json from dist folder into JSON',
-            e,
-        )
+        throw new Error('Failed to parse manifest.json into JSON')
     }
 
     try {
         fs.writeFileSync(normalizedManifestPath, manifest)
     } catch (e) {
         throw new Error(
             'Failed to write updated manifest.json to the dist folder',
-            e,
         )
     }
 
-    logger.log('Wrote version %s to %s', version, manifestPath)
+    logger.log('Wrote version %s to %s', version, normalizedManifestPath)
 }
 
 module.exports = {

src/publish.js

@@ -19,7 +19,7 @@ const publish = async options => {
         channel,
         sourceDir,
         targetXpi,
-    } = verifyConfig(options, ['extensionId'])
+    } = verifyConfig(options, ['extensionId', 'targetXpi'])
 
     const { FIREFOX_API_KEY, FIREFOX_SECRET_KEY } = process.env
     const { success, downloadedFiles } = await webExt.cmd.sign({

src/utils.js

@@ -1,6 +1,8 @@
-const throwErrors = errors => {
+const AggregateError = require('aggregate-error')
+
+const maybeThrowErrors = errors => {
     if (errors.length > 0) {
-        throw new Error(errors.join('\n'))
+        throw new AggregateError(errors)
     }
 }
 
@@ -19,11 +21,11 @@ const verifyConfig = (options, required = []) => {
             errors.push(`${prop} is missing from the options`)
         }
     })
-    throwErrors(errors)
+    maybeThrowErrors(errors)
     return mergedOptions
 }
 
 module.exports = {
-    throwErrors,
+    maybeThrowErrors,
     verifyConfig,
 }

src/verifyConditions.js

@@ -1,7 +1,7 @@
 const fs = require('fs')
 const path = require('path')
 
-const { throwErrors, verifyConfig } = require('./utils')
+const { maybeThrowErrors, verifyConfig } = require('./utils')
 
 const verifyConditions = options => {
     const verified = verifyConfig(options)
@@ -31,7 +31,7 @@ const verifyConditions = options => {
             `${manifestPath} was not found in ${sourceDir}, dist folder needs to exist to run`,
         )
     }
-    throwErrors(errors)
+    maybeThrowErrors(errors)
 }
 
 module.exports = {

tests/__mocks__/fs.js

@@ -0,0 +1,3 @@
+const { fs } = require('memfs')
+
+module.exports = fs

tests/__mocks__/web-ext.js

@@ -0,0 +1,6 @@
+const mockWebExt = {
+    cmd: {
+        sign: jest.fn(),
+    },
+}
+module.exports = { default: mockWebExt }

tests/prepare.test.js

@@ -0,0 +1,77 @@
+const { fs, vol } = require('memfs')
+
+const { prepare } = require('../src')
+
+describe('prepare', () => {
+    const mockOptions = {
+        sourceDir: 'mock_dist',
+        manifestPath: 'mock_manifest.json',
+    }
+    const defaultConfig = {
+        nextRelease: { version: 'v3.2.1' },
+        logger: console,
+    }
+
+    beforeAll(() => {
+        jest.spyOn(console, 'log')
+    })
+    afterEach(() => {
+        vol.reset()
+    })
+    afterAll(() => {
+        jest.restoreAllMocks()
+    })
+
+    it('uses default options when nothing supplied', () => {
+        vol.fromJSON({ 'dist/manifest.json': '{}' })
+        prepare({}, defaultConfig)
+        expect(console.log).toHaveBeenCalledWith(
+            'Wrote version %s to %s',
+            defaultConfig.nextRelease.version,
+            'dist/manifest.json',
+        )
+    })
+
+    it('uses supplied configuration options', () => {
+        vol.fromJSON({ 'mock_dist/mock_manifest.json': '{}' })
+        prepare(mockOptions, defaultConfig)
+        expect(console.log).toHaveBeenCalledWith(
+            'Wrote version %s to %s',
+            defaultConfig.nextRelease.version,
+            'mock_dist/mock_manifest.json',
+        )
+    })
+
+    it('fails if cannot read manifest file', () => {
+        expect(() => prepare(mockOptions, defaultConfig)).toThrowError(
+            'Unable to read manifest.json file',
+        )
+    })
+
+    it('fails if cannot parse manifest file', () => {
+        vol.fromJSON({ 'dist/manifest.json': 'this is not valid json' })
+        expect(() => prepare({}, defaultConfig)).toThrowError(
+            'Failed to parse manifest.json',
+        )
+    })
+
+    it('fails if cannot update manifest file', () => {
+        jest.spyOn(fs, 'readFileSync').mockImplementationOnce(() => '{}')
+        expect(() =>
+            prepare({ sourceDir: 'sourceDir' }, defaultConfig),
+        ).toThrowError('Failed to write updated manifest.json')
+    })
+
+    it.each([
+        [{}, 'dist/manifest.json'],
+        [mockOptions, 'mock_dist/mock_manifest.json'],
+    ])('successfully updates manifest file', (options, manifestPath) => {
+        vol.fromJSON({ [manifestPath]: '{}' })
+        prepare(options, defaultConfig)
+        expect(JSON.parse(fs.readFileSync(manifestPath))).toEqual(
+            expect.objectContaining({
+                version: 'v3.2.1',
+            }),
+        )
+    })
+})

tests/publish.test.js

@@ -0,0 +1,60 @@
+const { fs, vol } = require('memfs')
+const { default: webExt } = require('web-ext')
+
+const { publish } = require('../src')
+
+describe('publish', () => {
+    const extensionId = '{01234567-abcd-6789-cdef-0123456789ef}'
+    const targetXpi = 'target-extension.xpi'
+    const mockOptions = {
+        artifactsDir: 'mock_artifacts',
+        manifestPath: 'mock_manifest.json',
+        sourceDir: 'mock_source',
+    }
+    const completeOptions = { extensionId, targetXpi, ...mockOptions }
+
+    beforeAll(() => {
+        jest.spyOn(console, 'log')
+    })
+    afterEach(() => {
+        vol.reset()
+        jest.clearAllMocks()
+    })
+    afterAll(() => {
+        jest.restoreAllMocks()
+    })
+
+    it('fails if extensionId is not given', () => {
+        return expect(publish(mockOptions)).rejects.toThrow(
+            'extensionId is missing',
+        )
+    })
+
+    it('fails if targetXpi is not given', () => {
+        return expect(publish(mockOptions)).rejects.toThrow(
+            'targetXpi is missing',
+        )
+    })
+
+    it('raises error if signing unsuccessful', () => {
+        webExt.cmd.sign.mockResolvedValueOnce({ success: false })
+        return expect(publish(completeOptions)).rejects.toThrow(
+            'Signing the extension failed',
+        )
+    })
+
+    it('renames downloaded file to target xpi', async () => {
+        const downloadedFile = 'mock_downloaded.xpi'
+        vol.fromJSON({
+            [`${mockOptions.artifactsDir}/${downloadedFile}`]: 'some fake signed xpi',
+        })
+        webExt.cmd.sign.mockResolvedValueOnce({
+            success: true,
+            downloadedFiles: [downloadedFile],
+        })
+        const targetXpiPath = `${mockOptions.artifactsDir}/${targetXpi}`
+        expect(fs.existsSync(targetXpiPath)).toBe(false)
+        await publish(completeOptions)
+        expect(fs.existsSync(targetXpiPath)).toBe(true)
+    })
+})

tests/setup.js

@@ -0,0 +1,2 @@
+jest.mock('fs')
+jest.mock('web-ext')

tests/verifyConditions.test.js

@@ -0,0 +1,64 @@
+const { vol } = require('memfs')
+const mockProps = require('jest-mock-props')
+
+const { verifyConditions } = require('../src')
+
+mockProps.extend(jest)
+
+describe('verifyConditions', () => {
+    const firefoxApiKeySpy = jest.spyOnProp(process.env, 'FIREFOX_API_KEY')
+    const firefoxSecretKeySpy = jest.spyOnProp(
+        process.env,
+        'FIREFOX_SECRET_KEY',
+    )
+    const extensionId = '{01234567-abcd-6789-cdef-0123456789ef}'
+    const targetXpi = 'target-extension.xpi'
+
+    beforeEach(() => {
+        firefoxApiKeySpy.mockValueOnce('some-api-key')
+        firefoxSecretKeySpy.mockValueOnce('shh-its-a-secret')
+    })
+    afterEach(() => {
+        jest.resetAllMocks()
+    })
+    afterAll(() => {
+        jest.restoreAllMocks()
+    })
+
+    it('fails if FIREFOX_API_KEY is missing from env', () => {
+        firefoxApiKeySpy.mockReset()
+        expect(() => verifyConditions({ extensionId, targetXpi })).toThrow(
+            'FIREFOX_API_KEY is missing',
+        )
+    })
+
+    it('fails if FIREFOX_SECRET_KEY is missing from env', () => {
+        firefoxSecretKeySpy.mockReset()
+        expect(() => verifyConditions({ extensionId, targetXpi })).toThrow(
+            'FIREFOX_SECRET_KEY is missing',
+        )
+    })
+
+    it('fails if extensionId is missing from options', () => {
+        expect(() => verifyConditions({ targetXpi })).toThrow(
+            'No extensionId was specified',
+        )
+    })
+
+    it('fails if targetXpi is missing from options', () => {
+        expect(() => verifyConditions({ extensionId })).toThrow(
+            'No targetXpi was specified',
+        )
+    })
+
+    it('fails if manifest.json file does not exist', () => {
+        expect(() => verifyConditions({ extensionId, targetXpi })).toThrow(
+            'manifest.json was not found',
+        )
+    })
+
+    it('succeeds if all conditions are met', () => {
+        vol.fromJSON({ 'dist/manifest.json': '{}' })
+        expect(() => verifyConditions({ extensionId, targetXpi })).not.toThrow()
+    })
+})