Skip to content

Commit 9917909

Browse files
committed
jenkins: wait until status has been pushed to GitHub before responding
These changes are primarily done so we can get more reliable integration tests. Previously the incoming requests was responded to, before we knew whether or not the Jenkins status has been successfully pushed to GitHub. That makes it challenging to know when we can assert what we want to verify in integration tests, as we don't know when the entire operation has finished. Although the previous approach did what it should in practise when run in production, it should be just as important to have reliable tests that we can rely on when doing changes going forward.
1 parent 28d8a86 commit 9917909

File tree

2 files changed

+23
-14
lines changed

2 files changed

+23
-14
lines changed

lib/push-jenkins-update.js

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const url = require('url')
44

55
const githubClient = require('./github-client')
66

7-
function pushStarted (options, build) {
7+
function pushStarted (options, build, cb) {
88
const pr = findPrInRef(build.ref)
99

1010
// create unique logger which is easily traceable for every subsequent log statement
@@ -15,7 +15,9 @@ function pushStarted (options, build) {
1515

1616
findLatestCommitInPr(optsWithPr, (err, latestCommit) => {
1717
if (err) {
18-
return logger.error(err, 'Got error when retrieving GitHub commits for PR')
18+
logger.error(err, 'Got error when retrieving GitHub commits for PR')
19+
cb(err)
20+
return
1921
}
2022

2123
const statusOpts = Object.assign({
@@ -26,11 +28,11 @@ function pushStarted (options, build) {
2628
message: build.message || 'running tests'
2729
}, options)
2830

29-
createGhStatus(statusOpts, logger)
31+
createGhStatus(statusOpts, logger, cb)
3032
})
3133
}
3234

33-
function pushEnded (options, build) {
35+
function pushEnded (options, build, cb) {
3436
const pr = findPrInRef(build.ref)
3537

3638
// create unique logger which is easily traceable for every subsequent log statement
@@ -41,7 +43,9 @@ function pushEnded (options, build) {
4143

4244
findLatestCommitInPr(optsWithPr, (err, latestCommit) => {
4345
if (err) {
44-
return logger.error(err, 'Got error when retrieving GitHub commits for PR')
46+
logger.error(err, 'Got error when retrieving GitHub commits for PR')
47+
cb(err)
48+
return
4549
}
4650

4751
const statusOpts = Object.assign({
@@ -52,7 +56,7 @@ function pushEnded (options, build) {
5256
message: build.message || 'all tests passed'
5357
}, options)
5458

55-
createGhStatus(statusOpts, logger)
59+
createGhStatus(statusOpts, logger, cb)
5660
})
5761
}
5862

@@ -89,7 +93,7 @@ function findLatestCommitInPr (options, cb, pageNumber = 1) {
8993
})
9094
}
9195

92-
function createGhStatus (options, logger) {
96+
function createGhStatus (options, logger, cb) {
9397
githubClient.repos.createStatus({
9498
owner: options.owner,
9599
repo: options.repo,
@@ -100,9 +104,12 @@ function createGhStatus (options, logger) {
100104
description: options.message
101105
}, (err, res) => {
102106
if (err) {
103-
return logger.error(err, 'Error while updating Jenkins / GitHub PR status')
107+
logger.error(err, 'Error while updating Jenkins / GitHub PR status')
108+
cb(err)
109+
return
104110
}
105111
logger.info('Jenkins / Github PR status updated')
112+
cb(null)
106113
})
107114
}
108115

scripts/jenkins-status.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,10 @@ module.exports = function (app) {
4646
owner: 'nodejs',
4747
repo,
4848
logger: req.log
49-
}, req.body)
50-
51-
res.status(201).end()
49+
}, req.body, (err) => {
50+
const statusCode = err !== null ? 500 : 201
51+
res.status(statusCode).end()
52+
})
5253
})
5354

5455
app.post('/:repo/jenkins/end', (req, res) => {
@@ -75,8 +76,9 @@ module.exports = function (app) {
7576
owner: 'nodejs',
7677
repo,
7778
logger: req.log
78-
}, req.body)
79-
80-
res.status(201).end()
79+
}, req.body, (err) => {
80+
const statusCode = err !== null ? 500 : 201
81+
res.status(statusCode).end()
82+
})
8183
})
8284
}

0 commit comments

Comments
 (0)