Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: semver like titles #125

Merged
merged 7 commits into from
Dec 8, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ Possible options are:

For more details on how semantic version difference is calculated please see [semver](https://www.npmjs.com/package/semver) package.

If you set a value other than `any`, PRs that are not semantic version compliant will not be merged.
An example of a non-semantic version is a commit hash.

### `pr-number`

_Optional_ A pull request number, only required if triggered from a workflow_dispatch event. Typically this would be triggered by a script running in a seperate CI provider. See [Trigger action from workflow_dispatch event](#trigger-action-from-workflow_dispatch-event)
Expand Down
63 changes: 62 additions & 1 deletion dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6142,6 +6142,64 @@ class SemVer {
module.exports = SemVer


/***/ }),

/***/ 3466:
/***/ ((module, __unused_webpack_exports, __nccwpck_require__) => {

const SemVer = __nccwpck_require__(8088)
const parse = __nccwpck_require__(5925)
const {re, t} = __nccwpck_require__(9523)

const coerce = (version, options) => {
if (version instanceof SemVer) {
return version
}

if (typeof version === 'number') {
version = String(version)
}

if (typeof version !== 'string') {
return null
}

options = options || {}

let match = null
if (!options.rtl) {
match = version.match(re[t.COERCE])
} else {
// Find the right-most coercible string that does not share
// a terminus with a more left-ward coercible string.
// Eg, '1.2.3.4' wants to coerce '2.3.4', not '3.4' or '4'
//
// Walk through the string checking with a /g regexp
// Manually set the index so as to pick up overlapping matches.
// Stop when we get a match that ends at the string end, since no
// coercible string can be more right-ward without the same terminus.
let next
while ((next = re[t.COERCERTL].exec(version)) &&
(!match || match.index + match[0].length !== version.length)
) {
if (!match ||
next.index + next[0].length !== match.index + match[0].length) {
match = next
}
re[t.COERCERTL].lastIndex = next.index + next[1].length + next[2].length
}
// leave it in a clean state
re[t.COERCERTL].lastIndex = -1
}

if (match === null)
return null

return parse(`${match[2]}.${match[3] || '0'}.${match[4] || '0'}`, options)
}
module.exports = coerce


/***/ }),

/***/ 4309:
Expand Down Expand Up @@ -9179,6 +9237,7 @@ function isMajorRelease(pullRequest) {
"use strict";

const semverDiff = __nccwpck_require__(4297)
const semverCoerce = __nccwpck_require__(3466)

const { semanticVersionOrder } = __nccwpck_require__(5013)

Expand All @@ -9190,7 +9249,9 @@ const checkTargetMatchToPR = (prTitle, target) => {
if (!match) {
return true
}
const diff = semverDiff(match[1], match[2])

const [, from, to] = match
const diff = semverDiff(semverCoerce(from), semverCoerce(to))

return !(
diff &&
Expand Down
16 changes: 15 additions & 1 deletion src/checkTargetMatchToPR.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'
const semverDiff = require('semver/functions/diff')
const semverCoerce = require('semver/functions/coerce')

const { semanticVersionOrder } = require('./getTargetInput')

Expand All @@ -11,11 +12,24 @@ const checkTargetMatchToPR = (prTitle, target) => {
if (!match) {
return true
}
const diff = semverDiff(match[1], match[2])

const [, from, to] = match

const isNotSemantic = isAlpha(from)

|| isAlpha(to)


const diff = semverDiff(semverCoerce(from), semverCoerce(to))

return !(
diff &&
semanticVersionOrder.indexOf(diff) > semanticVersionOrder.indexOf(target)
)
}

function isAlpha(version) {
return /[A-Z]+/i.test(version)
}

module.exports = checkTargetMatchToPR
26 changes: 26 additions & 0 deletions test/action.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,29 @@ tap.test('should call external api for github-action-merge-dependabot major rele
t.ok(stubs.logStub.logWarning.calledOnce)
t.ok(stubs.fetchStub.calledOnce)
})

tap.test('should check submodules semver when target is set', async t => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not what submodules look like. submodules have a commit sha in the pr title (iirc) #95

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Eomm lmk if the comment is clear. It's about the title of the PR you have in this test not matching what happens when a version of a submodule tries to be bumped. You only get a SHA in that case

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm on it. Let me push the wip code

I'm trying to ignore the case:

  • the PR title is not a semver and the user set the target value

this check is a bit tricky due the prerelease stuff and the coercion.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't have to support everything, it would be a good step if we didn't break anything of the existing and we document the behavior for edge cases so we know what we can work on next

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have implemented the most simple check, using this PR as reference:

sitiom/dotfiles#1

const PR_NUMBER = Math.random()
const { action, stubs } = buildStubbedAction({
payload: {
pull_request: {
number: PR_NUMBER,
title: 'Bump dotbot from aa93350 to ac5793c',
user: { login: BOT_NAME },
head: { ref: 'dependabot/submodules/dotbot-ac5793c' },
}
},
inputs: {
PR_NUMBER,
TARGET: 'minor',
EXCLUDE_PKGS: [],
API_URL: 'custom one',
DEFAULT_API_URL,
}
})

await action()

t.ok(stubs.logStub.logInfo.calledOnceWith('pr-text'))
t.ok(stubs.fetchStub.calledOnce)
})
34 changes: 34 additions & 0 deletions test/checkTargetMatchToPR.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ const preReleaseToPathUpgradePRTitle =
'chore(deps-dev): bump fastify from 3.18.0-alpha to 3.18.2'
const sameVersion = 'chore(deps-dev): bump fastify from 3.18.0 to 3.18.0'
const patchPRTitleInSubDirectory = 'chore(deps-dev): bump fastify from 3.18.0 to 3.18.1 in /packages/a'
const semverLikeMinor = 'chore(deps): bump nearform/optic-release-automation-action from 2.2.0 to 2.3'
const semverLikeMajor = 'chore(deps): bump nearform/optic-release-automation-action from 2.2.0 to 3'
const semverLikeBothWay = 'chore(deps): bump nearform/optic-release-automation-action from 2 to 3'
const submodules = 'Bump dotbot from aa93350 to ac5793c'

tap.test('checkTargetMatchToPR', async t => {
t.test('should return true when target is major', async t => {
Expand Down Expand Up @@ -140,4 +144,34 @@ tap.test('checkTargetMatchToPR', async t => {
t.notOk(checkTargetMatchToPR(preMajorPRTitle, targetOptions.minor))
})
})

t.test('semver-like PR titles', async t => {
t.test('semver to minor semver-like', async t => {
t.notOk(checkTargetMatchToPR(semverLikeMinor, targetOptions.prepatch))
t.notOk(checkTargetMatchToPR(semverLikeMinor, targetOptions.patch))
t.ok(checkTargetMatchToPR(semverLikeMinor, targetOptions.minor))
t.ok(checkTargetMatchToPR(semverLikeMinor, targetOptions.major))
})

t.test('semver to major semver-like', async t => {
t.notOk(checkTargetMatchToPR(semverLikeMajor, targetOptions.prepatch))
t.notOk(checkTargetMatchToPR(semverLikeMajor, targetOptions.patch))
t.notOk(checkTargetMatchToPR(semverLikeMajor, targetOptions.minor))
t.ok(checkTargetMatchToPR(semverLikeMajor, targetOptions.major))
})

t.test('semver-like to semver-like', async t => {
t.notOk(checkTargetMatchToPR(semverLikeBothWay, targetOptions.prepatch))
t.notOk(checkTargetMatchToPR(semverLikeBothWay, targetOptions.patch))
t.notOk(checkTargetMatchToPR(semverLikeBothWay, targetOptions.minor))
t.ok(checkTargetMatchToPR(semverLikeBothWay, targetOptions.major))
})
})

t.test('submodules', async t => {
t.notOk(checkTargetMatchToPR(submodules, targetOptions.prepatch))
t.notOk(checkTargetMatchToPR(submodules, targetOptions.patch))
t.notOk(checkTargetMatchToPR(submodules, targetOptions.minor))
t.notOk(checkTargetMatchToPR(submodules, targetOptions.major))
})
})