Merge branch 'improve-tests'

lib/config.js

@@ -0,0 +1,6 @@
+const path = require('path')
+
+module.exports = {
+    S3_REGION: 'eu-west-1',
+    PGDUMP_PATH: path.join(__dirname, '../bin/postgres-9.6.2')
+}

lib/handler.js

@@ -1,22 +1,20 @@
 const utils = require('./utils')
 const encryption = require('./encryption')
-
+const Promise = require('bluebird')
 // todo: make these const, (mockSpawn doesn't allow this, so remove mockSpawn)
 var uploadS3 = require('./upload-s3')
 var pgdump = require('./pgdump')
 
-const DEFAULT_CONFIG = {
-    S3_REGION: 'eu-west-1'
-}
+const DEFAULT_CONFIG = require('./config')
 
-module.exports = function (event, context, cb) {
+function handler(event, context) {
     const config = Object.assign({}, DEFAULT_CONFIG, event)
 
     if (!config.PGDATABASE) {
-        return cb('PGDATABASE not provided in the event data')
+        throw new Error('PGDATABASE not provided in the event data')
     }
     if (!config.S3_BUCKET) {
-        return cb('S3_BUCKET not provided in the event data')
+        throw new Error('S3_BUCKET not provided in the event data')
     }
 
     // determine the path for the database dump
@@ -25,23 +23,32 @@ module.exports = function (event, context, cb) {
         config.ROOT
     )
 
-    // spawn pg_dump process
     const pgdumpProcess = pgdump(config)
-
     return pgdumpProcess
     .then(readableStream => {
         if (config.ENCRYPTION_PASSWORD) {
             console.log('encrypting dump')
-            readableStream = encryption.encrypt(readableStream, config.ENCRYPTION_PASSWORD)
+            readableStream = encryption.encrypt(
+                readableStream,
+                config.ENCRYPTION_PASSWORD
+            )
         }
         // stream to s3 uploader
         return uploadS3(readableStream, config, key)
-        .then(() => {
-            cb(null)
-        })
     })
     .catch(e => {
-        console.error(e)
-        cb(e)
+        throw e
+    })
+}
+
+module.exports = function (event, context, cb) {
+    return Promise.try(() => handler(event, context))
+    .then(result => {
+        cb(null)
+        return result
+    })
+    .catch(err => {
+        cb(err)
+        throw err
     })
 }

lib/pgdump.js

@@ -5,28 +5,29 @@ const fs = require('fs')
 
 function spawnPgDump(config) {
     const pgDumpPath = path.join(
-        config.PGDUMP_PATH || './bin/postgres-9.6.2',
+        config.PGDUMP_PATH,
         'pg_dump'
     )
     if (!fs.existsSync(pgDumpPath)) {
         throw new Error('pg_dump not found at ' + pgDumpPath)
     }
     const env = Object.assign({}, config, {
-        LD_LIBRARY_PATH: path.join(__dirname, '../bin/postgres-9.6.2')
+        LD_LIBRARY_PATH: config.PGDUMP_PATH
     })
     return spawn(pgDumpPath, ['-Fc'], {
         env
     })
 }
 
-function pgdumpWrapper(config, pgdumpFn = spawnPgDump) {
+function pgdumpWrapper(config, pgDumpSpawnFn = spawnPgDump) {
     return new Promise((resolve, reject) => {
-        let backupStarted = false
+        let headerChecked = false
         let stderr = ''
 
-        // spawn pg_dump process and attach hooks
-        const process = pgdumpFn(config)
+        // spawn pg_dump process
+        const process = pgDumpSpawnFn(config)
 
+        // hook into the process
         process.stderr.on('data', (data) => {
             stderr += data.toString('utf8')
         })
@@ -38,11 +39,10 @@ function pgdumpWrapper(config, pgdumpFn = spawnPgDump) {
                     new Error('pg_dump process failed: ' + stderr)
                 )
             }
-            // otherwise a zero exit is good
-            // check that pgdump gave us an expected response
-            if (!backupStarted) {
+            // check that pgdump actually gave us some data
+            if (!headerChecked) {
                 return reject(
-                    new Error('pg_dump didnt send us a recognizable dump')
+                    new Error('pg_dump gave us an unexpected response')
                 )
             }
             return null
@@ -53,12 +53,21 @@ function pgdumpWrapper(config, pgdumpFn = spawnPgDump) {
         const buffer = through2(function (chunk, enc, callback) {
             this.push(chunk)
             // if stdout begins with 'PGDMP' then the backup has begun
-            if (!backupStarted && chunk.toString('utf8').startsWith('PGDMP')) {
-                backupStarted = true
-                resolve(buffer)
+            // otherwise, we abort
+            if (!headerChecked) {
+                headerChecked = true
+                if (chunk.toString('utf8').startsWith('PGDMP')) {
+                    resolve(buffer)
+                }
+                else {
+                    reject(
+                        new Error('pg_dump gave us an unexpected response')
+                    )
+                }
             }
             callback()
         })
+
         // pipe pg_dump to buffer
         process.stdout.pipe(buffer)
     })

lib/upload-s3.js

@@ -5,8 +5,8 @@ AWS.config.update({
     logger: process.stdout
 })
 
-module.exports = function upload(stream, config, key) {
-    if (typeof stream.on !== 'function') {
+module.exports = function (stream, config, key) {
+    if (!stream || typeof stream.on !== 'function') {
         throw new Error('invalid stream provided')
     }
     return new Promise((resolve, reject) => {

package.json

@@ -4,6 +4,7 @@
   "description": "Lambda function for executing pg_dump and streaming the output to s3.",
   "main": "index.js",
   "dependencies": {
+    "bluebird": "^3.5.0",
     "moment": "^2.18.1",
     "through2": "^2.0.3"
   },
@@ -24,9 +25,10 @@
     "sinon": "^2.1.0"
   },
   "scripts": {
-    "test": "nyc --reporter=html --reporter=text mocha test",
+    "test": "mocha test",
     "test:watch": "mocha test -w",
     "coverage": "nyc report --reporter=text-lcov | coveralls",
+    "coverage-html": "nyc --reporter=html --reporter=text mocha test",
     "deploy": "bash bin/makezip.sh"
   },
   "repository": {

test/handler.js

@@ -1,51 +1,69 @@
+/* eslint no-underscore-dangle: 0 */
 const expect = require('chai').expect
 const rewire = require('rewire')
 const sinon = require('sinon')
 const mockSpawn = require('mock-spawn')
+const chai = require('chai')
+const chaiAsPromised = require('chai-as-promised')
 
-const handler = rewire('../lib/handler')
+chai.should()
+chai.use(chaiAsPromised)
 
+const handler = rewire('../lib/handler')
+const pgdump = require('../lib/pgdump')
 
 describe('Handler', () => {
-    const mockpgdump = () => {
+    const mockPgDumpSuccess = () => {
         const pgdumpProcess = mockSpawn()()
         pgdumpProcess.stdout.write('asdfasdf')
-        pgdumpProcess.stderr.write('some-error')
         pgdumpProcess.emit('close', 0)
         return Promise.resolve(pgdumpProcess)
     }
-    const mocks3upload = (stream, config, key) => {
-        return Promise.resolve('s3-key')
+    const mockS3UploadSuccess = (stream, config, key) => {
+        return Promise.resolve('mock-uploaded' + key)
     }
-    it('should backup', () => {
-        const s3Spy = sinon.spy(mocks3upload)
-        const pgSpy = sinon.spy(mockpgdump)
+
+    const mockEvent = {
+        PGDATABASE: 'dbname',
+        S3_BUCKET: 's3bucket'
+    }
+
+    function makeMockHandler({ mockPgdump, mockS3upload } = {}) {
+        mockPgdump = mockPgdump || mockPgDumpSuccess
+        mockS3upload = mockS3upload || mockS3UploadSuccess
+        const s3Spy = sinon.spy(mockS3upload)
+        const pgSpy = sinon.spy(mockPgdump)
         handler.__set__('pgdump', pgSpy)
         handler.__set__('uploadS3', s3Spy)
-
-        const event = {
-            PGDATABASE: 'dbname',
-            S3_BUCKET: 's3bucket'
+        return {
+            s3Spy,
+            pgSpy
         }
+    }
+
+
+    it('should backup', () => {
+        const { s3Spy, pgSpy } = makeMockHandler()
+
         const context = {}
         const cb = sinon.spy()
 
-        return handler(event, context, cb)
+        return handler(mockEvent, context, cb)
         .then(() => {
             // handler should have called pgSpy with correct arguments
             expect(pgSpy.calledOnce).to.be.true
             expect(pgSpy.firstCall.args).to.have.length(1)
             const [arg0] = pgSpy.firstCall.args
-            expect(arg0.S3_BUCKET).to.equal(event.S3_BUCKET)
-            expect(arg0.PGDATABASE).to.equal(event.PGDATABASE)
+            expect(arg0.S3_BUCKET).to.equal(mockEvent.S3_BUCKET)
+            expect(arg0.PGDATABASE).to.equal(mockEvent.PGDATABASE)
 
             // handler should have called s3spy with correct arguments
             expect(s3Spy.calledOnce).to.be.true
             expect(s3Spy.firstCall.args).to.have.length(3)
             const [stream, config, key] = s3Spy.firstCall.args
             expect(stream).to.be.ok
-            expect(config.S3_BUCKET).to.equal(event.S3_BUCKET)
-            expect(config.PGDATABASE).to.equal(event.PGDATABASE)
+            expect(config.S3_BUCKET).to.equal(mockEvent.S3_BUCKET)
+            expect(config.PGDATABASE).to.equal(mockEvent.PGDATABASE)
             expect(key).to.be.a.string
             expect(key).to.not.be.empty
 
@@ -55,4 +73,49 @@ describe('Handler', () => {
             expect(cb.firstCall.args[0]).to.be.null
         })
     })
+
+    it('should return an error when PGDATABASE is not provided', () => {
+        // remove PGDATABASE from the event config
+        makeMockHandler()
+        const event = Object.assign({}, mockEvent)
+        event.PGDATABASE = undefined
+
+        // call handler
+        const cb = sinon.spy()
+        return handler(event, {}, cb)
+        .should.be.rejectedWith(
+            /PGDATABASE not provided in the event data/
+        )
+    })
+
+    it('should return an error when S3_BUCKET is not provided', () => {
+        // remove S3_BUCKET from the event config
+        makeMockHandler()
+        const event = Object.assign({}, mockEvent)
+        event.S3_BUCKET = undefined
+
+        // call handler
+        const cb = sinon.spy()
+        return handler(event, {}, cb)
+        .should.be.rejectedWith(
+            /S3_BUCKET not provided in the event data/
+        )
+    })
+
+    it('should handle pgdump errors correctly', () => {
+        const pgdumpWithErrors = () => {
+            const pgdumpProcess = mockSpawn()()
+            pgdumpProcess.stderr.write('some-error')
+            pgdumpProcess.emit('close', 1)
+            return pgdumpProcess
+        }
+
+        makeMockHandler({
+            mockPgdump: () => pgdump(mockEvent, pgdumpWithErrors)
+        })
+
+        const cb = sinon.spy()
+        return handler(mockEvent, {}, cb)
+        .should.be.rejectedWith(/pg_dump gave us an unexpected response/)
+    })
 })

test/pgdump.js

@@ -1,11 +1,15 @@
-const pgdump = require('../lib/pgdump')
+const path = require('path')
+const fs = require('fs')
 const mockSpawn = require('mock-spawn')
 const chai = require('chai')
 const chaiAsPromised = require('chai-as-promised')
 
 chai.use(chaiAsPromised)
 const expect = chai.expect
 
+const pgdump = require('../lib/pgdump')
+const defaultConfig = require('../lib/config')
+
 describe('pgdump', () => {
     it('should export a function', () => {
         return expect(pgdump).to.be.a('function')
@@ -19,7 +23,9 @@ describe('pgdump', () => {
         pgdumpProcess.stdout.write('asdfasdf')
         pgdumpProcess.stderr.write('some-error')
         pgdumpProcess.emit('close', 0)
-        return expect(p).to.eventually.be.rejectedWith(/pg_dump didnt send us a recognizable dump/)
+        return expect(p).to.eventually.be.rejectedWith(
+            /pg_dump gave us an unexpected response/
+        )
     })
 
     it('should stream correctly', () => {
@@ -35,4 +41,23 @@ describe('pgdump', () => {
             expect(buffer.read().toString('utf8')).to.equal('PGDMP - data - data')
         })
     })
+    describe('default pg_dump binary', () => {
+        const binaryPath = path.join(defaultConfig.PGDUMP_PATH, 'pg_dump')
+        it('should exist', () => {
+            if (!fs.existsSync(binaryPath)) {
+                throw new Error('failed to find pg_dump at ', binaryPath)
+            }
+        })
+        it('should be +x', () => {
+            const fd = fs.openSync(binaryPath, 'r')
+            const stat = fs.fstatSync(fd)
+
+            // eslint-disable-next-line no-bitwise
+            const permString = '0' + (stat.mode & 0o777).toString(8)
+            expect(permString).to.equal('0777')
+            if (permString !== '0777') {
+                throw new Error('binary ' + binaryPath + ' is not executable')
+            }
+        })
+    })
 })