Skip to content

Commit

Permalink
feat: Add last comment validator (#668)
Browse files Browse the repository at this point in the history
* feat: Add last comment validator

* Update changelog

Co-authored-by: Cuong Pham <[email protected]>
  • Loading branch information
cuong-tech and Cuong Pham authored Sep 30, 2022
1 parent 7b8f60f commit 3c3fe59
Show file tree
Hide file tree
Showing 8 changed files with 189 additions and 29 deletions.
33 changes: 7 additions & 26 deletions __tests__/unit/actions/merge.test.js
Original file line number Diff line number Diff line change
@@ -1,30 +1,6 @@
const Merge = require('../../../lib/actions/merge')
const Helper = require('../../../__fixtures__/unit/helper')

test('check that merge is called if PR has not been merged', async () => {
const merge = new Merge()
const checkIfMerged = false
const context = Helper.mockContext({ checkIfMerged })
context.octokit.pulls.get.mockReturnValue({ data: { mergeable_state: 'clean', state: 'open' } })
const settings = {}

await merge.afterValidate(context, settings)
expect(context.octokit.pulls.merge.mock.calls.length).toBe(1)
expect(context.octokit.pulls.merge.mock.calls[0][0].merge_method).toBe('merge')
})

test('check that merge is called for review events', async () => {
const merge = new Merge()
const checkIfMerged = false
const context = Helper.mockContext({ checkIfMerged, eventName: 'pull_request_review' })
context.octokit.pulls.get.mockReturnValue({ data: { mergeable_state: 'clean', state: 'open' } })
const settings = {}

await merge.afterValidate(context, settings)
expect(context.octokit.pulls.merge.mock.calls.length).toBe(1)
expect(context.octokit.pulls.merge.mock.calls[0][0].merge_method).toBe('merge')
})

test('check that merge is called for status events', async () => {
const merge = new Merge()
const checkIfMerged = false
Expand All @@ -43,10 +19,15 @@ test('check that merge is called for status events', async () => {
expect(context.octokit.pulls.merge.mock.calls[0][0].merge_method).toBe('merge')
})

test('check that merge is called for check suite events', async () => {
test.each([
undefined,
'pull_request_review',
'check_suite',
'issue_comment'
])('check that merge is called for %s events', async (eventName) => {
const merge = new Merge()
const checkIfMerged = false
const context = Helper.mockContext({ checkIfMerged, eventName: 'check_suite' })
const context = Helper.mockContext({ checkIfMerged, eventName })
context.octokit.pulls.get.mockReturnValue({ data: { mergeable_state: 'clean', state: 'open' } })
const settings = {}

Expand Down
126 changes: 126 additions & 0 deletions __tests__/unit/validators/lastComment.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
const LastComment = require('../../../lib/validators/lastComment')
const Helper = require('../../../__fixtures__/unit/helper')

test('validate returns correctly', async () => {
const lastComment = new LastComment()

const settings = {
do: 'lastComment',
must_exclude: {
regex: 'exclude this'
}
}

let results = await lastComment.processValidate(createMockContext(['exclude this']), settings)
expect(results.status).toBe('fail')

results = await lastComment.processValidate(createMockContext(['a', 'b']), settings)
expect(results.status).toBe('pass')
})

test('fail gracefully if invalid regex', async () => {
const lastComment = new LastComment()

const settings = {
do: 'lastComment',
must_exclude: {
regex: '@#$@#$@#$'
}
}

const validation = await lastComment.processValidate(createMockContext('WIP'), settings)
expect(validation.status).toBe('pass')
})

test('description is correct', async () => {
const lastComment = new LastComment()

const settings = {
do: 'lastComment',
must_exclude: {
regex: 'Work in Progress'
}
}

let validation = await lastComment.processValidate(createMockContext('Work in Progress'), settings)

expect(validation.status).toBe('fail')
expect(validation.validations[0].description).toBe('lastComment does not exclude "Work in Progress"')

validation = await lastComment.processValidate(createMockContext('Just lastComment'), settings)
expect(validation.validations[0].description).toBe("lastComment must exclude 'Work in Progress'")
})

test('mergeable is true if must_include is the last comment', async () => {
const lastComment = new LastComment()

const settings = {
do: 'lastComment',
must_include: {
regex: 'xyz'
}
}

let validation = await lastComment.processValidate(createMockContext(['abc', 'experimental', 'xyz']), settings)
expect(validation.status).toBe('pass')

validation = await lastComment.processValidate(createMockContext(['Some lastComment', 'xyz', '456']), settings)
expect(validation.status).toBe('fail')
})

test('mergeable is false if must_exclude is one of the lastComment', async () => {
const lastComment = new LastComment()

const settings = {
do: 'lastComment',
must_exclude: {
regex: 'xyz'
}
}

let validation = await lastComment.processValidate(createMockContext(['abc', 'experimental', 'xyz']), settings)
expect(validation.status).toBe('fail')

validation = await lastComment.processValidate(createMockContext(['Some lastComment', 'xyz', '456']), settings)
expect(validation.status).toBe('pass')
})

test('complex Logic test', async () => {
const lastComment = new LastComment()

const settings = {
do: 'lastComment',
or: [{
and: [{
must_include: {
regex: 'release note: yes',
message: 'Please include release note: yes'
}
}, {
must_include: {
regex: 'lang\\/core|lang\\/c\\+\\+|lang\\/c#',
message: 'Please include a language comment'
}
}]
}, {
must_include: {
regex: 'release note: no',
message: 'Please include release note: no'
}
}]
}

let validation = await lastComment.processValidate(createMockContext(['experimental', 'xyz', 'release note: no']), settings)
expect(validation.status).toBe('pass')

validation = await lastComment.processValidate(createMockContext(['123', '456', 'release note: yes']), settings)
expect(validation.status).toBe('fail')
expect(validation.validations[0].description).toBe('((Please include a language comment) ***OR*** Please include release note: no)')

validation = await lastComment.processValidate(createMockContext(['456', 'lang/core']), settings)
expect(validation.validations[0].description).toBe('((Please include release note: yes) ***OR*** Please include release note: no)')
})

function createMockContext (comments) {
return Helper.mockContext({ listComments: Array.isArray(comments) ? comments.map(comment => ({ body: comment })) : [{ body: comments }] })
}
1 change: 1 addition & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
CHANGELOG
=====================================
| September 28, 2022: feat: Add last comment validator `#668 <https://github.com/mergeability/mergeable/pull/668>`_
| August 26, 2022: set fallback for `no_empty` options processor `#657 <https://github.com/mergeability/mergeable/pull/657>`_
| August 25, 2022: fix: set fallback value for `description` payload `#655 <https://github.com/mergeability/mergeable/pull/655>`_
| August 20, 2022: fix: supported events for `request_review` action `#651 <https://github.com/mergeability/mergeable/pull/651>`_
Expand Down
14 changes: 14 additions & 0 deletions docs/validators/last comment.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Last Comment
^^^^^^^^^^

::

- do: lastComment // check if the last comment contains only the word 'merge'
must_include:
regex: '^merge$'

Supported Events:
::

'pull_request.*', 'pull_request_review.*', 'issues.*', 'issue_comment.*'

3 changes: 2 additions & 1 deletion lib/actions/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class Merge extends Action {
'pull_request.*',
'pull_request_review.*',
'status.*',
'check_suite.*'
'check_suite.*',
'issue_comment.*'
]

this.supportedSettings = {
Expand Down
2 changes: 1 addition & 1 deletion lib/eventAware.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class EventAware {
return context.payload
}

if (context.eventName === 'issues') { // event name is 'issues' but payload contain 'issue'
if (context.eventName === 'issues' || context.eventName === 'issue_comment') {
return context.payload.issue
} else if (context.eventName === 'pull_request_review') {
return context.payload.pull_request
Expand Down
3 changes: 2 additions & 1 deletion lib/filters/payload.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ class Payload extends Filter {
this.supportedEvents = [
'pull_request.*',
'pull_request_review.*',
'issues.*'
'issues.*',
'issue_comment.*'
]
// no specific supported settings because it can vary by events
this.supportedSettings = {}
Expand Down
36 changes: 36 additions & 0 deletions lib/validators/lastComment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
const { Validator } = require('./validator')

class LastComment extends Validator {
constructor () {
super('lastComment')
this.supportedEvents = [
'pull_request.*',
'pull_request_review.*',
'issues.*',
'issue_comment.*'
]
this.supportedSettings = {
must_include: {
regex: ['string', 'array'],
regex_flag: 'string',
message: 'string'
},
must_exclude: {
regex: ['string', 'array'],
regex_flag: 'string',
message: 'string'
}
}
}

async validate (context, validationSettings) {
const comments = (await this.githubAPI.listComments(context, this.getPayload(context).number)).data

return this.processOptions(
validationSettings,
comments.length ? comments[comments.length - 1].body : ''
)
}
}

module.exports = LastComment

0 comments on commit 3c3fe59

Please sign in to comment.