Skip to content

Commit

Permalink
Merge test fixes into master (#331)
Browse files Browse the repository at this point in the history 
Resolve all failing unit and acceptance tests. Modernize handlePR controller with newer JS features.
  • Loading branch information
@alexwaibel
alexwaibel authored Feb 12, 2020
1 parent c8e339f commit 6f1f694
Show file tree
Hide file tree
Showing 11 changed files with 213 additions and 223 deletions.
19 changes: 8 additions & 11 deletions controllers/handlePR.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ module.exports = async (repo, data) => {
const github = await new GitHub({
username: data.repository.owner.login,
repository: data.repository.name,
token: config.get('githubToken')
version: '1'
})

return github.getReview(data.number).then(async (review) => {
try {
let review = await github.getReview(data.number)
if (review.sourceBranch.indexOf('staticman_')) {
return null
}
Expand All @@ -38,27 +39,23 @@ module.exports = async (repo, data) => {

staticman.setConfigPath(parsedBody.configPath)
staticman.processMerge(parsedBody.fields, parsedBody.options)
.catch(err => Promise.reject(err))
} catch (err) {
return Promise.reject(err)
}
}
}

return github.deleteBranch(review.sourceBranch)
}).then(response => {
if (ua) {
ua.event('Hooks', 'Delete branch').send()
}

return response
}).catch(err => {
console.log(err.stack || err)
return github.deleteBranch(review.sourceBranch)
} catch (e) {
console.log(e.stack || e)

if (ua) {
ua.event('Hooks', 'Delete branch error').send()
}

return Promise.reject(err)
})
return Promise.reject(e)
}
}
20 changes: 9 additions & 11 deletions index.js
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
(async () => {
try {
const StaticmanAPI = require('./server')
const api = await new StaticmanAPI()
try {
const StaticmanAPI = require('./server')
const api = new StaticmanAPI()

api.start(port => {
console.log('Staticman API running on port', port)
})
} catch (e) {
console.error(e)
}
})()
api.start(port => {
console.log('Staticman API running on port', port)
})
} catch (e) {
console.error(e)
}
3 changes: 2 additions & 1 deletion lib/ErrorHandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ const ErrorHandler = function () {
'RECAPTCHA_FAILED_DECRYPT': 'Could not decrypt reCAPTCHA secret',
'RECAPTCHA_CONFIG_MISMATCH': 'reCAPTCHA options do not match Staticman config',
'PARSING_ERROR': 'Error whilst parsing config file',
'GITHUB_AUTH_TOKEN_MISSING': 'The site requires a valid GitHub authentication token to be supplied in the `options[github-token]` field'
'GITHUB_AUTH_TOKEN_MISSING': 'The site requires a valid GitHub authentication token to be supplied in the `options[github-token]` field',
'MISSING_CONFIG_BLOCK': 'Error whilst parsing Staticman config file'
}

this.ERROR_CODE_ALIASES = {
Expand Down
4 changes: 2 additions & 2 deletions lib/GitHub.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ class GitHub extends GitService {
}

getReview (reviewId) {
return this.api.pullRequests.get({
return this.api.pulls.get({
owner: this.username,
repo: this.repository,
number: reviewId
pull_number: reviewId
})
.then(normalizeResponse)
.then(({base, body, head, merged, state, title}) =>
Expand Down
42 changes: 19 additions & 23 deletions server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,26 @@ const objectPath = require('object-path')

class StaticmanAPI {
constructor () {
return (async () => {
this.controllers = {
connect: require('./controllers/connect'),
encrypt: require('./controllers/encrypt'),
auth: require('./controllers/auth'),
handlePR: require('./controllers/handlePR'),
home: require('./controllers/home'),
process: require('./controllers/process')
}

this.server = express()
this.server.use(bodyParser.json())
this.server.use(bodyParser.urlencoded({
extended: true
// type: '*'
}))

this.initialiseWebhookHandler()
this.initialiseCORS()
this.initialiseBruteforceProtection()
this.initialiseRoutes()
this.controllers = {
connect: require('./controllers/connect'),
encrypt: require('./controllers/encrypt'),
auth: require('./controllers/auth'),
handlePR: require('./controllers/handlePR'),
home: require('./controllers/home'),
process: require('./controllers/process')
}

return this
})()
this.server = express()
this.server.use(bodyParser.json())
this.server.use(bodyParser.urlencoded({
extended: true
// type: '*'
}))

this.initialiseWebhookHandler()
this.initialiseCORS()
this.initialiseBruteforceProtection()
this.initialiseRoutes()
}

initialiseBruteforceProtection () {
Expand Down
22 changes: 12 additions & 10 deletions test/acceptance/api.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
const config = require('./../../config')
const config = require('../../config')
const githubToken = config.get('githubToken')
const helpers = require('./../helpers')
const helpers = require('../helpers')
const nock = require('nock')
const querystring = require('querystring')
const request = helpers.wrappedRequest
const sampleData = require('./../helpers/sampleData')
const StaticmanAPI = require('./../../server')
const sampleData = require('../helpers/sampleData')
const StaticmanAPI = require('../../server')

const btoa = contents => Buffer.from(contents).toString('base64')

let server

beforeAll(async (done) => {
server = await new StaticmanAPI()
beforeAll(done => {
server = new StaticmanAPI()

server.start(done)
server.start(() => {})

done()
})

afterAll(done => {
Expand Down Expand Up @@ -206,7 +208,7 @@ describe('Entry endpoint', () => {
})
})

test('outputs a PARSING_ERROR error the site config is malformed', () => {
test('outputs a MISSING_CONFIG_BLOCK error the site config is malformed', () => {
const data = Object.assign({}, helpers.getParameters(), {
path: 'staticman.yml'
})
Expand Down Expand Up @@ -252,8 +254,8 @@ describe('Entry endpoint', () => {
const error = JSON.parse(response.error)

expect(error.success).toBe(false)
expect(error.errorCode).toBe('PARSING_ERROR')
expect(error.message).toBeDefined()
expect(error.errorCode).toBe('MISSING_CONFIG_BLOCK')
expect(error.message).toBe('Error whilst parsing Staticman config file')
expect(error.rawError).toBeDefined()
})
})
Expand Down
10 changes: 5 additions & 5 deletions test/helpers/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ const SiteConfig = require('./../../siteConfig')
const yaml = require('js-yaml')

// Disable console.log() for tests
// if (process.env.TEST_DEV !== 'true') {
// console.debug = console.log
// console.log = jest.fn()
// console.warn = jest.fn()
// }
if (process.env.TEST_DEV !== 'true') {
console.debug = console.log
console.log = jest.fn()
console.warn = jest.fn()
}

const rsa = new NodeRSA()
rsa.importKey(config.get('rsaPrivateKey'), 'private')
Expand Down
86 changes: 45 additions & 41 deletions test/unit/controllers/auth.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ beforeEach(() => {

describe('Auth controller', () => {
describe('GitHub', () => {
test('authenticates to GitHub with the given code and returns the authenticated user', () => {
test('authenticates to GitHub with the given code and returns the authenticated user', async () => {
const mockAccessToken = 'qwertyuiop'
const mockCode = '1q2w3e4r'
const mockUser = {
Expand All @@ -41,11 +41,12 @@ describe('Auth controller', () => {
access_token: mockAccessToken
})

nock(/github\.com/)
nock((/github\.com/), {
reqheaders: {
authorization: `token ${mockAccessToken}`
}
})
.get('/user')
.query({
access_token: mockAccessToken
})
.reply(200, mockUser)

const reqWithQuery = Object.assign({}, req, {
Expand All @@ -54,15 +55,13 @@ describe('Auth controller', () => {
}
})

return auth(reqWithQuery, res).then(result => {
expect(res.send).toHaveBeenCalledTimes(1)
expect(helpers.decrypt(res.send.mock.calls[0][0].accessToken)).toBe(mockAccessToken)
expect(res.send.mock.calls[0][0].user)
.toEqual(new User('github', mockUser.login, mockUser.email, mockUser.name))
})
await auth(reqWithQuery, res)
expect(res.send).toHaveBeenCalledTimes(1)
expect(helpers.decrypt(res.send.mock.calls[0][0].accessToken)).toBe(mockAccessToken)
expect(res.send.mock.calls[0][0].user).toEqual(new User('github', mockUser.login, mockUser.email, mockUser.name))
})

test('authenticates to GitHub with the given code and returns the original GitHub user when using v2 API', () => {
test('authenticates to GitHub with the given code and returns the original GitHub user when using v2 API', async () => {
const mockAccessToken = 'qwertyuiop'
const mockCode = '1q2w3e4r'
const mockUser = {
Expand All @@ -83,12 +82,13 @@ describe('Auth controller', () => {
access_token: mockAccessToken
})

nock(/github\.com/)
.get('/user')
.query({
access_token: mockAccessToken
nock((/github\.com/), {
reqheaders: {
authorization: `token ${mockAccessToken}`
}
})
.reply(200, mockUser)
.get('/user')
.reply(200, mockUser)

const reqWithQuery = Object.assign({}, req, {
params: {
Expand All @@ -100,14 +100,13 @@ describe('Auth controller', () => {
}
})

return auth(reqWithQuery, res).then(result => {
expect(res.send).toHaveBeenCalledTimes(1)
expect(helpers.decrypt(res.send.mock.calls[0][0].accessToken)).toBe(mockAccessToken)
expect(res.send.mock.calls[0][0].user).toEqual(mockUser)
})
await auth(reqWithQuery, res)
expect(res.send).toHaveBeenCalledTimes(1)
expect(helpers.decrypt(res.send.mock.calls[0][0].accessToken)).toBe(mockAccessToken)
expect(res.send.mock.calls[0][0].user).toEqual(mockUser)
})

test('returns a 401 response when unable to get an access token from GitHub', () => {
test('returns a 401 response when unable to get an access token from GitHub', async () => {
const mockCode = '1q2w3e4r'
const siteConfig = helpers.getConfig()

Expand All @@ -125,21 +124,21 @@ describe('Auth controller', () => {

const reqWithQuery = Object.assign({}, req, {
params: {
service: 'github'
service: 'github',
version: '2'
},
query: {
code: mockCode
}
})

return auth(reqWithQuery, res).then(result => {
expect(res.status.mock.calls[0][0]).toBe(401)
expect(res.send.mock.calls[0][0].statusCode).toBe(401)
expect(res.send.mock.calls[0][0].message).toContain('invalid_code')
})
await auth(reqWithQuery, res)
expect(res.status.mock.calls[0][0]).toBe(401)
expect(res.send.mock.calls[0][0].statusCode).toBe(401)
expect(res.send.mock.calls[0][0].message).toContain('invalid_code')
})

test('returns a 401 response when an incorrect access token is used for the GitHub API', () => {
test('returns a 401 response when an incorrect access token is used for the GitHub API', async () => {
const mockAccessToken = 'qwertyuiop'
const mockCode = '1q2w3e4r'

Expand All @@ -157,25 +156,30 @@ describe('Auth controller', () => {
access_token: mockAccessToken
})

nock(/github\.com/).get('/user')
.query({
access_token: mockAccessToken
})
.reply(401, {
message: 'Unauthorized'
nock((/github\.com/), {
reqheaders: {
authorization: `token ${mockAccessToken}`
}
})
.get('/user')
.reply(401, {
message: 'Unauthorized'
})

const reqWithQuery = Object.assign({}, req, {
params: {
service: 'github',
version: '2'
},
query: {
code: mockCode
}
})

return auth(reqWithQuery, res).then(result => {
expect(res.status.mock.calls[0][0]).toBe(401)
expect(res.send.mock.calls[0][0].statusCode).toBe(401)
expect(res.send.mock.calls[0][0].message).toContain('Unauthorized')
})
await auth(reqWithQuery, res)
expect(res.status.mock.calls[0][0]).toBe(401)
expect(res.send.mock.calls[0][0].statusCode).toBe(401)
expect(res.send.mock.calls[0][0].message).toContain('Unauthorized')
})
})

Expand Down
Loading

0 comments on commit 6f1f694

Please sign in to comment.