fix: improve PR Title parsing, handle invalid inputs (#186)

* fix: improve PR Title parsing, handle invalid inputs

Use branch name to get package name, avoid issues with namespacing.
Fail when parsing invalid PR title.

* refactor: extract getPackageName function and add unit test

* test: improve getPackageName tests

dist/index.js

@@ -9299,7 +9299,7 @@ const toolkit = __nccwpck_require__(2183)
 const packageInfo = __nccwpck_require__(4147)
 const { githubClient } = __nccwpck_require__(3386)
 const { logInfo, logWarning, logError } = __nccwpck_require__(653)
-const { getInputs } = __nccwpck_require__(6254)
+const { getInputs, getPackageName } = __nccwpck_require__(6254)
 const { targetOptions } = __nccwpck_require__(5013)
 const {
   getModuleVersionChanges,
@@ -9384,23 +9384,20 @@ function isAMajorReleaseBump(change) {
   const from = change.delete
   const to = change.insert
 
-  if (!from || !to) {
-    return false
-  }
-
   const diff = semverDiff(semverCoerce(from), semverCoerce(to))
   return diff === targetOptions.major
 }
 
 function parsePrTitle(pullRequest) {
-  const expression = /bump (\S+) from (\S+) to (\S+)/i
+  const expression = /bump \S+ from (\S+) to (\S+)/i
   const match = expression.exec(pullRequest.title)
 
   if (!match) {
-    return {}
+    throw new Error('Error while parsing PR title, expected: `bump <package> from <old-version> to <new-version>`')
   }
+  const [, oldVersion, newVersion] = match
 
-  const [, packageName, oldVersion, newVersion] = match
+  const packageName = getPackageName(pullRequest.head.ref)
 
   return { [packageName]: { delete: semverCoerce(oldVersion).raw, insert: semverCoerce(newVersion).raw } }
 }
@@ -9666,6 +9663,23 @@ exports.getInputs = () => ({
   PR_NUMBER: core.getInput('pr-number'),
 })
 
+/**
+ * Get a package name from a branch name.
+ * Dependabot branch names are in format "dependabot/npm_and_yarn/pkg-0.0.1"
+ * or "dependabot/github_actions/fastify/github-action-merge-dependabot-2.6.0"
+ */
+exports.getPackageName = (branchName) => {
+  const nameWithVersion = branchName.split('/').pop().split('-')
+  const version = nameWithVersion.pop()
+  const packageName = nameWithVersion.join('-')
+
+  if (!packageName || !version) {
+    throw new Error('Invalid branch name, package name or version not found')
+  }
+
+  return packageName
+}
+
 
 /***/ }),
 

src/action.js

@@ -9,7 +9,7 @@ const toolkit = require('actions-toolkit')
 const packageInfo = require('../package.json')
 const { githubClient } = require('./github-client')
 const { logInfo, logWarning, logError } = require('./log')
-const { getInputs } = require('./util')
+const { getInputs, getPackageName } = require('./util')
 const { targetOptions } = require('./getTargetInput')
 const {
   getModuleVersionChanges,
@@ -94,23 +94,20 @@ function isAMajorReleaseBump(change) {
   const from = change.delete
   const to = change.insert
 
-  if (!from || !to) {
-    return false
-  }
-
   const diff = semverDiff(semverCoerce(from), semverCoerce(to))
   return diff === targetOptions.major
 }
 
 function parsePrTitle(pullRequest) {
-  const expression = /bump (\S+) from (\S+) to (\S+)/i
+  const expression = /bump \S+ from (\S+) to (\S+)/i
   const match = expression.exec(pullRequest.title)
 
   if (!match) {
-    return {}
+    throw new Error('Error while parsing PR title, expected: `bump <package> from <old-version> to <new-version>`')
   }
+  const [, oldVersion, newVersion] = match
 
-  const [, packageName, oldVersion, newVersion] = match
+  const packageName = getPackageName(pullRequest.head.ref)
 
   return { [packageName]: { delete: semverCoerce(oldVersion).raw, insert: semverCoerce(newVersion).raw } }
 }

src/util.js

@@ -37,3 +37,20 @@ exports.getInputs = () => ({
   TARGET: getTargetInput(core.getInput('target')),
   PR_NUMBER: core.getInput('pr-number'),
 })
+
+/**
+ * Get a package name from a branch name.
+ * Dependabot branch names are in format "dependabot/npm_and_yarn/pkg-0.0.1"
+ * or "dependabot/github_actions/fastify/github-action-merge-dependabot-2.6.0"
+ */
+exports.getPackageName = (branchName) => {
+  const nameWithVersion = branchName.split('/').pop().split('-')
+  const version = nameWithVersion.pop()
+  const packageName = nameWithVersion.join('-')
+
+  if (!packageName || !version) {
+    throw new Error('Invalid branch name, package name or version not found')
+  }
+
+  return packageName
+}

test/action.test.js

@@ -297,7 +297,10 @@ tap.test('should merge major bump using PR title', async () => {
       pull_request: {
         number: PR_NUMBER,
         user: { login: BOT_NAME },
-        title: 'build(deps): bump actions/checkout from 2 to 3'
+        title: 'build(deps): bump actions/checkout from 2 to 3',
+        head: {
+          ref: 'dependabot/github_actions/actions/checkout-3'
+        }
       }
     },
     inputs: {
@@ -323,7 +326,10 @@ tap.test('should forbid major bump using PR title', async () => {
       pull_request: {
         number: PR_NUMBER,
         user: { login: BOT_NAME },
-        title: 'build(deps): bump actions/checkout from 2 to 3'
+        title: 'build(deps): bump actions/checkout from 2 to 3',
+        head: {
+          ref: 'dependabot/github_actions/actions/cache-3'
+        }
       }
     },
     inputs: {
@@ -350,7 +356,10 @@ tap.test('should not merge major bump if updating github-action-merge-dependabot
       pull_request: {
         number: PR_NUMBER,
         user: { login: BOT_NAME },
-        title: 'build(deps): bump github-action-merge-dependabot from 2 to 3'
+        title: 'build(deps): bump github-action-merge-dependabot from 2 to 3 zzz',
+        head: {
+          ref: 'dependabot/github_actions/fastify/github-action-merge-dependabot-3'
+        }
       }
     },
     inputs: {
@@ -376,7 +385,10 @@ tap.test('should throw if the PR title is not valid', async () => {
       pull_request: {
         number: PR_NUMBER,
         user: { login: BOT_NAME },
-        title: 'Invalid PR title'
+        title: 'Invalid PR title',
+        head: {
+          ref: 'dependabot/github_actions/fastify/github-action-merge-dependabot-2.6.0'
+        }
       }
     },
     inputs: {
@@ -389,32 +401,8 @@ tap.test('should throw if the PR title is not valid', async () => {
   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)
+  sinon.assert.calledWith(stubs.coreStub.setFailed, ("Error while parsing PR title, expected: `bump <package> from <old-version> to <new-version>`"))
+  sinon.assert.notCalled(stubs.approveStub)
+  sinon.assert.notCalled(stubs.mergeStub)
 })

test/util.test.js

@@ -1,5 +1,6 @@
 'use strict'
 const tap = require('tap')
+const { getPackageName } = require('../src/util')
 
 const coreStubs = {
   'getInput': () => '',
@@ -27,3 +28,15 @@ tap.test('MERGE_METHOD should be correct for valid input', async t => {
   })
   t.equal(getInputs().MERGE_METHOD, 'merge')
 })
+
+tap.test('getPackageName should get package name from branch with repo name', async t => {
+  t.equal(getPackageName("dependabot/github_actions/fastify/github-action-merge-dependabot-2.6.0"), "github-action-merge-dependabot")
+})
+
+tap.test('getPackageName should get package name from branch', async t => {
+  t.equal(getPackageName("dependabot/npm_and_yarn/pkg-0.0.1"), "pkg")
+})
+
+tap.test('getPackageName should throw an error for invalid branch names', async t => {
+  t.throws(() => getPackageName("invalidbranchname"), new Error('Invalid branch name, package name or version not found'))
+})