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

chore: update changelog linting #25809

Merged
merged 2 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion scripts/semantic-commits/get-binary-release-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const getReleaseData = async (latestReleaseInfo) => {
return
}

const { data: pullRequest } = await octokit.request('GET /repos/{owner}/{repo}/pull/{pull_number}', {
const { data: pullRequest } = await octokit.request('GET /repos/{owner}/{repo}/pulls/{pull_number}', {
Copy link
Contributor

Choose a reason for hiding this comment

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

so octokit does pulls when the conventional url is pull?

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 appear so: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request
made the mistake of updating to pull which broken this in develop.

owner: 'cypress-io',
repo: 'cypress',
pull_number: references[0].issue,
Expand Down
2 changes: 1 addition & 1 deletion scripts/semantic-commits/get-linked-issues.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const getLinkedIssues = (body = '') => {
// remove markdown comments
body.replace(/(<!--.*?-->)|(<!--[\S\s]+?-->)|(<!--[\S\s]*?$)/g, '')

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
// find linked issues. see: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
// match on: #issue_number, cypress-io/cypress#issue_number and https://github.com/cypress-io/cypress/issues/issue_number

const references = body.match(/(close[sd]?|fix(es|ed)?|resolve[s|d]?) (cypress-io\/cypress)?#\d+/gi)
const references = body.match(/(close[sd]?|fix(es|ed)?|resolve[s|d]?) ((cypress-io\/cypress)?#\d+|https\:\/\/github.com\/cypress-io\/cypress\/issues\/\d+)/gi)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be worth giving some examples of what this is trying to match in a comment?


if (!references) {
return []
Expand Down
52 changes: 36 additions & 16 deletions scripts/semantic-commits/validate-changelog.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,40 @@ function _getResolvedMessage (semanticType, prNumber, associatedIssues = []) {
return `[#${issueNumber}](https://github.com/cypress-io/cypress/issues/${issueNumber})`
})

// one issue: [#num]
// two issues: [#num] and [#num]
// two+ issues: [#num], [#num] and [#num]
const linkMessage = [links.slice(0, -1).join(', '), links.slice(-1)[0]].join(links.length < 2 ? '' : ' and ')

return `${issueMessage} ${linkMessage}.`
return {
message: issueMessage,
links,
}
}

const prMessage = userFacingChanges[semanticType].message.onlyPR

return `${prMessage} [#${prNumber}](https://github.com/cypress-io/cypress/pull/${prNumber}).`
return {
message: prMessage,
links: [`[#${prNumber}](https://github.com/cypress-io/cypress/pull/${prNumber})`],
}
}

function _linksText (links) {
// one issue: [#num]
// two issues: [#num] and [#num]
// two+ issues: [#num], [#num] and [#num]
const linkMessage = [links.slice(0, -1).join(', '), links.slice(-1)[0]].join(links.length < 2 ? '' : ' and ')

return linkMessage
}

function _printResolveExample ({ message, links }) {
return `${message} ${_linksText(links)}.`
}

/**
* Helper to format an example of what the changelog entry might look like for a given commit.
*/
function _printChangeLogExample (semanticType, prNumber, associatedIssues = []) {
const resolveMessage = _getResolvedMessage(semanticType, prNumber, associatedIssues)
const resolveData = _getResolvedMessage(semanticType, prNumber, associatedIssues)

return `${userFacingChanges[semanticType].section}\n\n - <Insert change details>. ${resolveMessage}`
return `${userFacingChanges[semanticType].section}\n\n - <Insert change details>. ${_printResolveExample(resolveData)}`
}

/**
Expand All @@ -51,21 +65,27 @@ function _validateEntry (changelog, { commitMessage, prNumber, semanticType, ass
}

const expectedSection = userFacingChanges[semanticType].section
let missingExpectedSection = false
let missingExpectedSection = !changelog[expectedSection]
let sectionEntryFoundIn = ''

const resolveMessage = _getResolvedMessage(semanticType, prNumber, associatedIssues)
const resolveData = _getResolvedMessage(semanticType, prNumber, associatedIssues)

const hasMatchingEntry = Object.entries(userFacingChanges).some(([type, { section }]) => {
const sectionDetails = changelog[section]

if (!sectionDetails) {
missingExpectedSection = semanticType === type

return false
}

const hasMatchingEntry = sectionDetails.some((detail) => detail.includes(resolveMessage))
const hasMatchingEntry = sectionDetails.some((detail) => {
const index = detail.lastIndexOf(resolveData.message)

if (index === undefined) return false // missing message

const resolveString = detail.substring(index)

return resolveData.links.every((link) => resolveString.includes(link))
})

if (hasMatchingEntry) {
sectionEntryFoundIn = section
Expand All @@ -80,10 +100,10 @@ function _validateEntry (changelog, { commitMessage, prNumber, semanticType, ass

if (!hasMatchingEntry) {
if (associatedIssues && associatedIssues.length) {
return `The changelog entry does not include the linked issues that this pull request resolves. Please update your entry for '${commitMessage}' to include:\n\n${resolveMessage}`
return `The changelog entry does not include the linked issues that this pull request resolves. Please update your entry for '${commitMessage}' to include:\n\n${_printResolveExample(resolveData)}`
}

return `The changelog entry does not include the pull request link. Please update your entry for '${commitMessage}' to include:\n\n${resolveMessage}`
return `The changelog entry does not include the pull request link. Please update your entry for '${commitMessage}' to include:\n\n${_printResolveExample(resolveData)}`
}

if (hasMatchingEntry && sectionEntryFoundIn !== expectedSection) {
Expand Down
58 changes: 50 additions & 8 deletions scripts/unit/semantic-commits/validate-changelog-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,36 @@ use(sinonChai)
describe('semantic-pull-request/validate-changelog', () => {
context('_getResolvedMessage', () => {
it('returned pr link', () => {
const message = _getResolvedMessage('feat', 52, [])
const { message, links } = _getResolvedMessage('feat', 52, [])

expect(message).to.contain('Addressed in [#52](https://github.com/cypress-io/cypress/pull/52).')
expect(message).to.eq('Addressed in')
expect(links).to.have.length(1)
expect(links[0]).to.eq('[#52](https://github.com/cypress-io/cypress/pull/52)')
})

it('returns linked issue', () => {
const message = _getResolvedMessage('feat', 52, [39])
const { message, links } = _getResolvedMessage('feat', 52, [39])

expect(message).to.contain('Addresses [#39](https://github.com/cypress-io/cypress/issues/39).')
expect(message).to.eq('Addresses')
expect(links).to.have.length(1)
expect(links[0]).to.eq('[#39](https://github.com/cypress-io/cypress/issues/39)')
})

it('returns all linked issues', () => {
let message = _getResolvedMessage('feat', 52, [39, 20])
let { message, links } = _getResolvedMessage('feat', 52, [39, 20])

expect(message).to.contain('Addresses [#20](https://github.com/cypress-io/cypress/issues/20) and [#39](https://github.com/cypress-io/cypress/issues/39).')
expect(message).to.eq('Addresses')
expect(links).to.have.length(2)
expect(links[0]).to.eq('[#20](https://github.com/cypress-io/cypress/issues/20)')
expect(links[1]).to.eq('[#39](https://github.com/cypress-io/cypress/issues/39)')

message = _getResolvedMessage('feat', 52, [39, 20, 30])
const resolveData = _getResolvedMessage('feat', 52, [39, 20, 30])

expect(message).to.contain('Addresses [#20](https://github.com/cypress-io/cypress/issues/20), [#30](https://github.com/cypress-io/cypress/issues/30) and [#39](https://github.com/cypress-io/cypress/issues/39).')
expect(resolveData.message).to.eq('Addresses')
expect(resolveData.links).to.have.length(3)
expect(resolveData.links[0]).to.eq('[#20](https://github.com/cypress-io/cypress/issues/20)')
expect(resolveData.links[1]).to.eq('[#30](https://github.com/cypress-io/cypress/issues/30)')
expect(resolveData.links[2]).to.eq('[#39](https://github.com/cypress-io/cypress/issues/39)')
})
})

Expand Down Expand Up @@ -140,6 +151,37 @@ _Released 01/17/2033 (PENDING)_
expect(console.log).to.be.calledWith('It appears at a high-level your changelog entry is correct! The remaining validation is left to the pull request reviewers.')
})

it('verifies changelog with shared entry', async () => {
const changedFiles = [
'packages/driver/lib/index.js',
'cli/CHANGELOG.md',
]

fs.readFileSync.returns(`
## 120.2.0

_Released 01/17/2033 (PENDING)_

**Misc:**

- Addresses [#77](https://github.com/cypress-io/cypress/issues/77) and [#88](https://github.com/cypress-io/cypress/issues/88).`)

await validateChangelog({
changedFiles,
commits: [{
prNumber: 74,
semanticType: 'misc',
associatedIssues: ['77'],
}, {
prNumber: 75,
semanticType: 'misc',
associatedIssues: ['88'],
}],
})

expect(console.log).to.be.calledWith('It appears at a high-level your changelog entry is correct! The remaining validation is left to the pull request reviewers.')
})

describe('ignores validation', () => {
it('when commit has cli or binary file changes that are not user facing', async () => {
const changedFiles = [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
const { expect, use } = require('chai')
const sinonChai = require('sinon-chai')

const { getIssueNumbers } = require('../../semantic-commits/get-linked-issues')
const { getLinkedIssues } = require('../../semantic-commits/get-linked-issues')

use(sinonChai)

describe('semantic-commits/get-linked-issues', () => {
it('returns single issue link', () => {
const issues = getIssueNumbers(`
const issues = getLinkedIssues(`
<!-- comment ->
- Closes #23
summary of changes see in #458
Expand All @@ -17,7 +17,7 @@ describe('semantic-commits/get-linked-issues', () => {
})

it('returns issue links for all linking keywords', () => {
const issues = getIssueNumbers(`
const issues = getLinkedIssues(`
<!-- comment ->
- Close #23
- Closes #24
Expand All @@ -29,17 +29,18 @@ describe('semantic-commits/get-linked-issues', () => {
- Resolves #46
- addresses #77 <-- not a valid linking word
summary of changes
- Closes https://github.com/cypress-io/cypress/issues/50
`)

expect(issues).to.deep.eq(['23', '24', '25', '33', '34', '35', '44', '45', '46'])
expect(issues).to.deep.eq(['23', '24', '25', '33', '34', '35', '44', '45', '46', '50'])
})

it('only counts an issue once', () => {
const body = `
- closes #44
- closes #44
`
const issues = getIssueNumbers(body)
const issues = getLinkedIssues(body)

expect(issues).to.deep.eq(['44'])
})
Expand All @@ -49,13 +50,13 @@ describe('semantic-commits/get-linked-issues', () => {
fixes cypress-io/cypress#123 which is a local issue
and this is issue in another repo foo/bar#101
`
const issues = getIssueNumbers(body)
const issues = getLinkedIssues(body)

expect(issues).to.deep.eq(['123'])
})

it('returns empty list when no issues found', () => {
const issues = getIssueNumbers(`
const issues = getLinkedIssues(`
<!-- comment ->
summary of changes
`)
Expand Down