Skip to content

Commit cf9614f

Browse files
phillipjphillipj
committed
jenkins: update existing comment when CI comment has already been posted
The motivation behind these changes are to prevent flooding PRs with CI comments from the bot whenever new node-test-pull-request jobs are started. Instead if the bot has already posted a CI comment earlier, it will update the last CI comment and change the URL in that comment to the new job that got started.
1 parent f63962c commit cf9614f

File tree

5 files changed

+256
-4
lines changed

5 files changed

+256
-4
lines changed

‎lib/github-comment.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,16 @@ exports.createPrComment = function createPrComment ({ owner, repo, number, logge
1414
}
1515
})
1616
}
17+
18+
exports.editPrComment = function createPrComment ({ owner, repo, logger }, commentId, body) {
19+
githubClient.issues.editComment({
20+
owner,
21+
repo,
22+
id: commentId,
23+
body
24+
}, (err) => {
25+
if (err) {
26+
logger.error(err, 'Error while editing comment on GitHub')
27+
}
28+
})
29+
}

‎lib/push-jenkins-update.js‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
'use strict'
22

33
const url = require('url')
4+
const async = require('async')
45

56
const githubClient = require('./github-client')
6-
const { createPrComment } = require('./github-comment')
7+
const { createPrComment, editPrComment } = require('./github-comment')
8+
const botUsername = require('./bot-username')
79

810
function pushStarted (options, build, cb) {
911
const pr = findPrInRef(build.ref)
@@ -15,7 +17,19 @@ function pushStarted (options, build, cb) {
1517
const optsWithPr = Object.assign({ pr }, options)
1618

1719
if (build.identifier === 'node-test-pull-request' && build.status === 'pending') {
18-
createPrComment(Object.assign({ number: pr }, options), `CI: ${build.url}`)
20+
findExistingCiComment(optsWithPr, logger, (err, existingComment) => {
21+
if (err) {
22+
logger.error(err, 'Got error while retrieving GitHub comments for PR')
23+
return
24+
}
25+
26+
const body = `CI: ${build.url}`
27+
if (existingComment === undefined) {
28+
createPrComment(Object.assign({ number: pr }, options), body)
29+
} else {
30+
editPrComment(options, existingComment.id, body)
31+
}
32+
})
1933
}
2034

2135
findLatestCommitInPr(optsWithPr, (err, latestCommit) => {
@@ -118,6 +132,25 @@ function createGhStatus (options, logger, cb) {
118132
})
119133
}
120134

135+
function findExistingCiComment (options, logger, cb) {
136+
async.parallel([
137+
botUsername.resolve,
138+
githubClient.issues.getComments.bind(githubClient.issues, {
139+
owner: options.owner,
140+
repo: options.repo,
141+
number: options.pr
142+
})
143+
],
144+
(err, [username, commentsResponse]) => {
145+
if (err) {
146+
return cb(err)
147+
}
148+
149+
const comments = commentsResponse.data.reverse()
150+
cb(null, comments.find((comment) => comment.body.startsWith('CI: ') && comment.user.login === username))
151+
})
152+
}
153+
121154
function validate (payload) {
122155
const isString = (param) => typeof (payload[param]) === 'string'
123156
return ['identifier', 'status', 'message', 'commit', 'url'].every(isString)

‎test/_fixtures/authenticated-user.json‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
{
2+
"login": "nodejs-github-bot",
3+
"id": 18269663,
4+
"node_id": "MDQ6VXNlcjE4MjY5NjYz",
5+
"avatar_url": "https://avatars1.githubusercontent.com/u/18269663?v=4",
6+
"gravatar_id": "",
7+
"url": "https://api.github.com/users/nodejs-github-bot",
8+
"html_url": "https://github.com/nodejs-github-bot",
9+
"followers_url": "https://api.github.com/users/nodejs-github-bot/followers",
10+
"following_url": "https://api.github.com/users/nodejs-github-bot/following{/other_user}",
11+
"gists_url": "https://api.github.com/users/nodejs-github-bot/gists{/gist_id}",
12+
"starred_url": "https://api.github.com/users/nodejs-github-bot/starred{/owner}{/repo}",
13+
"subscriptions_url": "https://api.github.com/users/nodejs-github-bot/subscriptions",
14+
"organizations_url": "https://api.github.com/users/nodejs-github-bot/orgs",
15+
"repos_url": "https://api.github.com/users/nodejs-github-bot/repos",
16+
"events_url": "https://api.github.com/users/nodejs-github-bot/events{/privacy}",
17+
"received_events_url": "https://api.github.com/users/nodejs-github-bot/received_events",
18+
"type": "User",
19+
"site_admin": false,
20+
"name": "Node.js GitHub Bot",
21+
"company": null,
22+
"blog": "https://github.com/nodejs/github-bot",
23+
"location": null,
24+
"email": null,
25+
"hireable": null,
26+
"bio": null,
27+
"public_repos": 1,
28+
"public_gists": 0,
29+
"followers": 21,
30+
"following": 0,
31+
"created_at": "2016-04-04T18:31:48Z",
32+
"updated_at": "2018-11-21T02:22:03Z"
33+
}

‎test/_fixtures/pull-request-comments-with-ci-comment.json‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
[
2+
{
3+
"url": "https://api.github.com/repos/nodejs/node/issues/comments/475182143",
4+
"html_url": "https://github.com/nodejs/node/pull/26837#issuecomment-475182143",
5+
"issue_url": "https://api.github.com/repos/nodejs/node/issues/26837",
6+
"id": 475182143,
7+
"node_id": "MDEyOklzc3VlQ29tbWVudDQ3NTE4MjE0Mw==",
8+
"user": {
9+
"login": "nodejs-github-bot",
10+
"id": 18269663,
11+
"node_id": "MDQ6VXNlcjE4MjY5NjYz",
12+
"avatar_url": "https://avatars1.githubusercontent.com/u/18269663?v=4",
13+
"gravatar_id": "",
14+
"url": "https://api.github.com/users/nodejs-github-bot",
15+
"html_url": "https://github.com/nodejs-github-bot",
16+
"followers_url": "https://api.github.com/users/nodejs-github-bot/followers",
17+
"following_url": "https://api.github.com/users/nodejs-github-bot/following{/other_user}",
18+
"gists_url": "https://api.github.com/users/nodejs-github-bot/gists{/gist_id}",
19+
"starred_url": "https://api.github.com/users/nodejs-github-bot/starred{/owner}{/repo}",
20+
"subscriptions_url": "https://api.github.com/users/nodejs-github-bot/subscriptions",
21+
"organizations_url": "https://api.github.com/users/nodejs-github-bot/orgs",
22+
"repos_url": "https://api.github.com/users/nodejs-github-bot/repos",
23+
"events_url": "https://api.github.com/users/nodejs-github-bot/events{/privacy}",
24+
"received_events_url": "https://api.github.com/users/nodejs-github-bot/received_events",
25+
"type": "User",
26+
"site_admin": false
27+
},
28+
"created_at": "2019-03-21T10:42:26Z",
29+
"updated_at": "2019-03-21T10:42:26Z",
30+
"author_association": "MEMBER",
31+
"body": "@addaleax build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2998/pipeline"
32+
},
33+
{
34+
"url": "https://api.github.com/repos/nodejs/node/issues/comments/475368357",
35+
"html_url": "https://github.com/nodejs/node/pull/26837#issuecomment-475368357",
36+
"issue_url": "https://api.github.com/repos/nodejs/node/issues/26837",
37+
"id": 475368357,
38+
"node_id": "MDEyOklzc3VlQ29tbWVudDQ3NTM2ODM1Nw==",
39+
"user": {
40+
"login": "addaleax",
41+
"id": 899444,
42+
"node_id": "MDQ6VXNlcjg5OTQ0NA==",
43+
"avatar_url": "https://avatars2.githubusercontent.com/u/899444?v=4",
44+
"gravatar_id": "",
45+
"url": "https://api.github.com/users/addaleax",
46+
"html_url": "https://github.com/addaleax",
47+
"followers_url": "https://api.github.com/users/addaleax/followers",
48+
"following_url": "https://api.github.com/users/addaleax/following{/other_user}",
49+
"gists_url": "https://api.github.com/users/addaleax/gists{/gist_id}",
50+
"starred_url": "https://api.github.com/users/addaleax/starred{/owner}{/repo}",
51+
"subscriptions_url": "https://api.github.com/users/addaleax/subscriptions",
52+
"organizations_url": "https://api.github.com/users/addaleax/orgs",
53+
"repos_url": "https://api.github.com/users/addaleax/repos",
54+
"events_url": "https://api.github.com/users/addaleax/events{/privacy}",
55+
"received_events_url": "https://api.github.com/users/addaleax/received_events",
56+
"type": "User",
57+
"site_admin": false
58+
},
59+
"created_at": "2019-03-21T19:20:30Z",
60+
"updated_at": "2019-03-21T19:20:30Z",
61+
"author_association": "MEMBER",
62+
"body": "CI: https://ci.nodejs.org/job/node-test-pull-request/21737/"
63+
},
64+
{
65+
"url": "https://api.github.com/repos/nodejs/node/issues/comments/475385809",
66+
"html_url": "https://github.com/nodejs/node/pull/26837#issuecomment-475385809",
67+
"issue_url": "https://api.github.com/repos/nodejs/node/issues/26837",
68+
"id": 475385809,
69+
"node_id": "MDEyOklzc3VlQ29tbWVudDQ3NTM4NTgwOQ==",
70+
"user": {
71+
"login": "addaleax",
72+
"id": 899444,
73+
"node_id": "MDQ6VXNlcjg5OTQ0NA==",
74+
"avatar_url": "https://avatars2.githubusercontent.com/u/899444?v=4",
75+
"gravatar_id": "",
76+
"url": "https://api.github.com/users/addaleax",
77+
"html_url": "https://github.com/addaleax",
78+
"followers_url": "https://api.github.com/users/addaleax/followers",
79+
"following_url": "https://api.github.com/users/addaleax/following{/other_user}",
80+
"gists_url": "https://api.github.com/users/addaleax/gists{/gist_id}",
81+
"starred_url": "https://api.github.com/users/addaleax/starred{/owner}{/repo}",
82+
"subscriptions_url": "https://api.github.com/users/addaleax/subscriptions",
83+
"organizations_url": "https://api.github.com/users/addaleax/orgs",
84+
"repos_url": "https://api.github.com/users/addaleax/repos",
85+
"events_url": "https://api.github.com/users/addaleax/events{/privacy}",
86+
"received_events_url": "https://api.github.com/users/addaleax/received_events",
87+
"type": "User",
88+
"site_admin": false
89+
},
90+
"created_at": "2019-03-21T20:16:21Z",
91+
"updated_at": "2019-03-21T20:16:21Z",
92+
"author_association": "MEMBER",
93+
"body": "Resume CI: https://ci.nodejs.org/job/node-test-pull-request/21739/"
94+
},
95+
{
96+
"url": "https://api.github.com/repos/nodejs/node/issues/comments/476584580",
97+
"html_url": "https://github.com/nodejs/node/pull/26837#issuecomment-476584580",
98+
"issue_url": "https://api.github.com/repos/nodejs/node/issues/26837",
99+
"id": 476584580,
100+
"node_id": "MDEyOklzc3VlQ29tbWVudDQ3NjU4NDU4MA==",
101+
"user": {
102+
"login": "nodejs-github-bot",
103+
"id": 18269663,
104+
"node_id": "MDQ6VXNlcjE4MjY5NjYz",
105+
"avatar_url": "https://avatars1.githubusercontent.com/u/18269663?v=4",
106+
"gravatar_id": "",
107+
"url": "https://api.github.com/users/nodejs-github-bot",
108+
"html_url": "https://github.com/nodejs-github-bot",
109+
"followers_url": "https://api.github.com/users/nodejs-github-bot/followers",
110+
"following_url": "https://api.github.com/users/nodejs-github-bot/following{/other_user}",
111+
"gists_url": "https://api.github.com/users/nodejs-github-bot/gists{/gist_id}",
112+
"starred_url": "https://api.github.com/users/nodejs-github-bot/starred{/owner}{/repo}",
113+
"subscriptions_url": "https://api.github.com/users/nodejs-github-bot/subscriptions",
114+
"organizations_url": "https://api.github.com/users/nodejs-github-bot/orgs",
115+
"repos_url": "https://api.github.com/users/nodejs-github-bot/repos",
116+
"events_url": "https://api.github.com/users/nodejs-github-bot/events{/privacy}",
117+
"received_events_url": "https://api.github.com/users/nodejs-github-bot/received_events",
118+
"type": "User",
119+
"site_admin": false
120+
},
121+
"created_at": "2019-03-26T11:25:56Z",
122+
"updated_at": "2019-03-26T11:25:56Z",
123+
"author_association": "MEMBER",
124+
"body": "CI: https://ci.nodejs.org/job/node-test-pull-request/21904/"
125+
}
126+
]

‎test/integration/push-jenkins-update.test.js‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,18 @@ tap.test('Forwards payload provided in incoming POST to GitHub status API', (t)
104104

105105
tap.test('Posts a CI comment in the related PR when Jenkins build is named node-test-pull-request', (t) => {
106106
const fixture = readFixture('jenkins-test-pull-request-success-payload.json')
107-
const commentScope = nock('https://api.github.com')
107+
const createCommentScope = nock('https://api.github.com')
108108
.filteringPath(ignoreQueryParams)
109109
.post('/repos/nodejs/node/issues/12345/comments', { body: 'CI: https://ci.nodejs.org/job/node-test-pull-request/21633/' })
110110
.reply(200)
111111

112+
nock('https://api.github.com')
113+
.filteringPath(ignoreQueryParams)
114+
.get('/repos/nodejs/node/issues/12345/comments')
115+
.reply(200, [])
116+
112117
// we don't care about asserting the scopes below, just want to stop the requests from actually being sent
118+
setupGetBotUsernameMock()
113119
setupGetCommitsMock('node')
114120
nock('https://api.github.com')
115121
.filteringPath(ignoreQueryParams)
@@ -123,7 +129,41 @@ tap.test('Posts a CI comment in the related PR when Jenkins build is named node-
123129
.send(fixture)
124130
.expect(201)
125131
.end((err, res) => {
126-
commentScope.done()
132+
createCommentScope.done()
133+
t.equal(err, null)
134+
})
135+
})
136+
137+
tap.test('Edits existing CI comment when bot has posted a CI comment before', (t) => {
138+
const incomingReqFixture = readFixture('jenkins-test-pull-request-success-payload.json')
139+
const existingCommentsFixture = readFixture('pull-request-comments-with-ci-comment.json')
140+
141+
const editCommentScope = nock('https://api.github.com')
142+
.filteringPath(ignoreQueryParams)
143+
.patch('/repos/nodejs/node/issues/comments/476584580', { body: 'CI: https://ci.nodejs.org/job/node-test-pull-request/21633/' })
144+
.reply(200)
145+
146+
nock('https://api.github.com')
147+
.filteringPath(ignoreQueryParams)
148+
.get('/repos/nodejs/node/issues/12345/comments')
149+
.reply(200, existingCommentsFixture)
150+
151+
// we don't care about asserting the scopes below, just want to stop the requests from actually being sent
152+
setupGetBotUsernameMock()
153+
setupGetCommitsMock('node')
154+
nock('https://api.github.com')
155+
.filteringPath(ignoreQueryParams)
156+
.post('/repos/nodejs/node/statuses/8a5fec2a6bade91e544a30314d7cf21f8a200de1')
157+
.reply(201)
158+
159+
t.plan(1)
160+
161+
supertest(app)
162+
.post('/node/jenkins/start')
163+
.send(incomingReqFixture)
164+
.expect(201)
165+
.end((err, res) => {
166+
editCommentScope.done()
127167
t.equal(err, null)
128168
})
129169
})
@@ -205,6 +245,13 @@ function setupGetCommitsMock (repoName) {
205245
.reply(200, commitsResponse)
206246
}
207247

248+
function setupGetBotUsernameMock () {
249+
nock('https://api.github.com')
250+
.filteringPath(ignoreQueryParams)
251+
.get('/user')
252+
.reply(200, readFixture('authenticated-user.json'))
253+
}
254+
208255
function ignoreQueryParams (pathAndQuery) {
209256
return url.parse(pathAndQuery, true).pathname
210257
}

0 commit comments

Comments
 (0)