Add missing tests (#180)

* test: add some of the missing tests

* chore: enforce 100% code coverage

* test: add missing github-client tests

* fix: handle invalid PR titles and add test case for it

* test: improve log module tests

* test: fix invalid PR assertion

* chore: remove .taprc file

All options that were declared there are default now.

* test: reduce util tests duplication

* test: use sinon.restore instead of iterating through stubs

* fix: undo behavior changes

Those changes will be added back in another PR if needed.

* test: add test case for packages with incomplete versions

dist/index.js

@@ -9383,6 +9383,7 @@ ${changedExcludedPackages.join(', ')}. Skipping.`)
 function isAMajorReleaseBump(change) {
   const from = change.delete
   const to = change.insert
+
   if (!from || !to) {
     return false
   }
@@ -9652,13 +9653,13 @@ const getMergeMethod = () => {
 }
 
 const parseCommaSeparatedValue = (value) => {
-  return value.split(',').map(el => el.trim());
+  return value ? value.split(',').map(el => el.trim()) : [];
 }
 
 exports.getInputs = () => ({
   GITHUB_TOKEN: core.getInput('github-token', { required: true }),
   MERGE_METHOD: getMergeMethod(),
-  EXCLUDE_PKGS: parseCommaSeparatedValue(core.getInput('exclude')) || [],
+  EXCLUDE_PKGS: parseCommaSeparatedValue(core.getInput('exclude')),
   MERGE_COMMENT: core.getInput('merge-comment') || '',
   APPROVE_ONLY: /true/i.test(core.getInput('approve-only')),
   TARGET: getTargetInput(core.getInput('target')),

src/action.js

@@ -93,6 +93,7 @@ ${changedExcludedPackages.join(', ')}. Skipping.`)
 function isAMajorReleaseBump(change) {
   const from = change.delete
   const to = change.insert
+
   if (!from || !to) {
     return false
   }

src/util.js

@@ -25,13 +25,13 @@ const getMergeMethod = () => {
 }
 
 const parseCommaSeparatedValue = (value) => {
-  return value.split(',').map(el => el.trim());
+  return value ? value.split(',').map(el => el.trim()) : [];
 }
 
 exports.getInputs = () => ({
   GITHUB_TOKEN: core.getInput('github-token', { required: true }),
   MERGE_METHOD: getMergeMethod(),
-  EXCLUDE_PKGS: parseCommaSeparatedValue(core.getInput('exclude')) || [],
+  EXCLUDE_PKGS: parseCommaSeparatedValue(core.getInput('exclude')),
   MERGE_COMMENT: core.getInput('merge-comment') || '',
   APPROVE_ONLY: /true/i.test(core.getInput('approve-only')),
   TARGET: getTargetInput(core.getInput('target')),

test/action.test.js

@@ -367,3 +367,54 @@ tap.test('should not merge major bump if updating github-action-merge-dependabot
   sinon.assert.notCalled(stubs.approveStub)
   sinon.assert.notCalled(stubs.mergeStub)
 })
+
+tap.test('should throw if the PR title is not valid', async () => {
+  const PR_NUMBER = Math.random()
+
+  const { action, stubs } = buildStubbedAction({
+    payload: {
+      pull_request: {
+        number: PR_NUMBER,
+        user: { login: BOT_NAME },
+        title: 'Invalid PR title'
+      }
+    },
+    inputs: {
+      PR_NUMBER,
+      TARGET: 'major',
+      EXCLUDE_PKGS: ['react'],
+    }
+  })
+
+  stubs.prDiffStub.resolves(diffs.noPackageJsonChanges)
+
+  await action()
+  sinon.assert.called(stubs.approveStub)
+  sinon.assert.called(stubs.mergeStub)
+})
+
+tap.test('should handle a newly added package', async () => {
+  const PR_NUMBER = Math.random()
+
+  const { action, stubs } = buildStubbedAction({
+    payload: {
+      pull_request: {
+        number: PR_NUMBER,
+        user: { login: BOT_NAME },
+        title: 'Invalid PR title'
+      }
+    },
+    inputs: {
+      PR_NUMBER,
+      TARGET: 'any',
+      EXCLUDE_PKGS: ['react'],
+    }
+  })
+
+  stubs.prDiffStub.resolves(diffs.thisModuleAdded)
+
+  await action()
+
+  sinon.assert.called(stubs.approveStub)
+  sinon.assert.called(stubs.mergeStub)
+})

test/github-client.test.js

@@ -0,0 +1,95 @@
+'use strict'
+
+const tap = require('tap');
+const sinon = require('sinon');
+
+const githubContext = {
+  repository: {
+    owner: {
+      login: 'owner-login.',
+    },
+    name: 'repository-name'
+  }
+}
+
+const data = 'octokit-result'
+
+const octokitStubs = {
+  get: sinon.stub().returns(Promise.resolve({ data })),
+  createReview: sinon.stub().returns(Promise.resolve({ data })),
+  merge: sinon.stub().returns(Promise.resolve({ data })),
+}
+
+const { githubClient } = tap.mock('../src/github-client', {
+  '@actions/github': {
+    context: {
+      payload: githubContext
+    },
+    getOctokit: () => ({
+      rest: {
+        pulls: octokitStubs
+      }
+    })
+  }
+})
+
+const TOKEN = 'GITHUB-TOKEN'
+const PR_NUMBER = Math.floor(Math.random() * 10);
+
+tap.afterEach(() => {
+  sinon.resetHistory();
+})
+
+tap.test('githubClient', async t => {
+  t.test('getPullRequest', async () => {
+    const result = await githubClient(TOKEN).getPullRequest(PR_NUMBER)
+    tap.equal(result, data)
+
+    sinon.assert.calledWith(octokitStubs.get, {
+      owner: githubContext.repository.owner.login,
+      repo: githubContext.repository.name,
+      pull_number: PR_NUMBER
+    })
+  })
+
+  t.test('approvePullRequest', async () => {
+    const comment = 'Test pull request comment'
+    const result = await githubClient(TOKEN).approvePullRequest(PR_NUMBER, comment)
+    tap.equal(result, data)
+
+    sinon.assert.calledWith(octokitStubs.createReview, {
+      owner: githubContext.repository.owner.login,
+      repo: githubContext.repository.name,
+      pull_number: PR_NUMBER,
+      event: 'APPROVE',
+      body: comment
+    })
+  })
+
+  t.test('mergePullRequest', async () => {
+    const method = 'squash'
+    const result = await githubClient(TOKEN).mergePullRequest(PR_NUMBER, method)
+    tap.equal(result, data)
+
+    sinon.assert.calledWith(octokitStubs.merge, {
+      owner: githubContext.repository.owner.login,
+      repo: githubContext.repository.name,
+      pull_number: PR_NUMBER,
+      merge_method: method,
+    })
+  })
+
+  t.test('getPullRequestDiff', async () => {
+    const result = await githubClient(TOKEN).getPullRequestDiff(PR_NUMBER)
+    tap.equal(result, data)
+
+    sinon.assert.calledWith(octokitStubs.get, {
+      owner: githubContext.repository.owner.login,
+      repo: githubContext.repository.name,
+      pull_number: PR_NUMBER,
+      mediaType: {
+        format: 'diff'
+      }
+    })
+  })
+})

test/log.test.js

@@ -0,0 +1,40 @@
+'use strict'
+
+const tap = require('tap')
+const sinon = require('sinon')
+
+const coreStubs = {
+  debug: sinon.stub(),
+  error: sinon.stub(),
+  info: sinon.stub(),
+  warning: sinon.stub(),
+}
+const log = tap.mock('../src/log', {
+  '@actions/core': coreStubs
+})
+
+tap.afterEach(() => {
+  sinon.resetHistory()
+})
+
+tap.test('logError should work with numbers', async () => {
+  log.logError(100)
+  sinon.assert.calledWith(coreStubs.error, '100')
+})
+
+tap.test('logError should work with strings', async () => {
+  log.logError('100')
+  sinon.assert.calledWith(coreStubs.error, '100')
+})
+
+tap.test('log should call aproppriate core function', async () => {
+  const msg = 'log message'
+  log.logError(msg)
+  sinon.assert.calledWith(coreStubs.error, msg)
+  log.logWarning(msg)
+  sinon.assert.calledWith(coreStubs.warning, msg)
+  log.logInfo(msg)
+  sinon.assert.calledWith(coreStubs.info, msg)
+  log.logDebug(msg)
+  sinon.assert.calledWith(coreStubs.debug, msg)
+})

test/moduleChanges.js

@@ -92,6 +92,14 @@ index d3dfd3d..bd28161 100644
 @@ -19,9 +19,9 @@
 -        "dotbot": "aa93350",
 +        "dotbot": "ac5793c",
+`,
+  thisModuleAdded: `
+diff --git a/package.json b/package.json
+index d3dfd3d..bd28161 100644
+--- a/package.json
++++ b/package.json
+@@ -19,9 +19,9 @@
++        "github-action-merge-dependabot": "1.4.0",
 `,
   multiplePackagesMajorMinor: `
 diff --git a/package.json b/package.json
@@ -146,6 +154,15 @@ index d3dfd3d..bd28161 100644
 @@ -19,9 +19,9 @@
 -        "github-action-merge-dependabot": "1.2.3",
 +        "github-action-merge-dependabot": "2.1.0",
+`,
+  thisModuleInvalidVersion: `
+diff --git a/package.json b/package.json
+index d3dfd3d..bd28161 100644
+--- a/package.json
++++ b/package.json
+@@ -19,9 +19,9 @@
+-        "github-action-merge-dependabot": "",
++        "github-action-merge-dependabot": "",
 `,
   noPackageJsonChanges: `
 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml

test/moduleVersionChanges.test.js

@@ -120,4 +120,8 @@ tap.test('checkModuleVersionChanges', async t => {
     t.notOk(checkModuleVersionChanges(moduleChanges.multipleFilesDiff, targetOptions.minor))
     t.ok(checkModuleVersionChanges(moduleChanges.multipleFilesDiff, targetOptions.major))
   })
+
+  t.test('should deal with invalid package version', async t => {
+    t.notOk(checkModuleVersionChanges({ 'github-action-merge-dependabot-': { insert: '', delete: '' } }, targetOptions.minor))
+  })
 })

test/util.test.js

@@ -0,0 +1,29 @@
+'use strict'
+const tap = require('tap')
+
+const coreStubs = {
+  'getInput': () => '',
+  debug: msg => msg,
+  error: msg => msg,
+  info: msg => msg,
+  warning: msg => msg,
+}
+
+tap.test('MERGE_METHOD should be squash for invalid input', async t => {
+  const { getInputs } = t.mock('../src/util', {
+    '@actions/core': {
+      ...coreStubs
+    }
+  })
+  t.equal(getInputs().MERGE_METHOD, 'squash')
+})
+
+tap.test('MERGE_METHOD should be correct for valid input', async t => {
+  const { getInputs } = tap.mock('../src/util', {
+    '@actions/core': {
+      ...coreStubs,
+      'getInput': () => 'merge',
+    }
+  })
+  t.equal(getInputs().MERGE_METHOD, 'merge')
+})