From f981c1c7701ef5387203021bc2f9116d21b134ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Mon, 22 Jan 2018 13:16:43 -0200 Subject: [PATCH 01/10] WIP --- lib/plugins/aws/deploy/lib/uploadArtifacts.js | 54 ++++++++++--------- .../aws/deploy/lib/uploadArtifacts.test.js | 46 +++++++++++++--- 2 files changed, 66 insertions(+), 34 deletions(-) diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.js index a6b21e770..2c9baa953 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.js @@ -2,6 +2,7 @@ /* eslint-disable no-use-before-define */ +const _ = require('lodash'); const fs = require('fs'); const path = require('path'); const crypto = require('crypto'); @@ -80,36 +81,37 @@ module.exports = { }, uploadFunctions() { - let shouldUploadService = false; this.serverless.cli.log('Uploading artifacts...'); + const functionNames = this.serverless.service.getAllFunctions(); - return BbPromise.map(functionNames, (name) => { - const functionArtifactFileName = this.provider.naming.getFunctionArtifactName(name); - const functionObject = this.serverless.service.getFunction(name); - functionObject.package = functionObject.package || {}; - let artifactFilePath = functionObject.package.artifact || - this.serverless.service.package.artifact; - if (!artifactFilePath || - (this.serverless.service.artifact && !functionObject.package.artifact)) { - if (this.serverless.service.package.individually || functionObject.package.individually) { - const artifactFileName = functionArtifactFileName; - artifactFilePath = path.join(this.packagePath, artifactFileName); - return this.uploadZipFile(artifactFilePath); + const artifactFilePaths = _.uniq( + _.map(functionNames, (name) => { + const functionArtifactFileName = this.provider.naming.getFunctionArtifactName(name); + const functionObject = this.serverless.service.getFunction(name); + functionObject.package = functionObject.package || {}; + const artifact = functionObject.package.artifact || + this.serverless.service.package.artifact; + console.log(`artifactFilePath ${JSON.stringify(artifact, null, 2)}`) + if (!artifact || + (this.serverless.service.artifact && !functionObject.package.artifact)) { + if (this.serverless.service.package.individually || functionObject.package.individually) { + const artifactFileName = functionArtifactFileName; + return path.join(this.packagePath, artifactFileName); + } + return this.provider.naming.getServiceArtifactName(); } - shouldUploadService = true; - return BbPromise.resolve(); - } + return artifact; + }) + ); + + console.log(`artifactFileNames ${JSON.stringify(artifactFilePaths, null, 2)}`) + + return BbPromise.map(artifactFilePaths, (artifactFilePath) => { + const stats = fs.statSync(artifactFilePath); + this.serverless.cli.log(`Uploading service .zip file to S3 (${filesize(stats.size)})...`); return this.uploadZipFile(artifactFilePath); - }, { concurrency: 3 }).then(() => { - if (shouldUploadService) { - const artifactFileName = this.provider.naming.getServiceArtifactName(); - const artifactFilePath = path.join(this.packagePath, artifactFileName); - const stats = fs.statSync(artifactFilePath); - this.serverless.cli.log(`Uploading service .zip file to S3 (${filesize(stats.size)})...`); - return this.uploadZipFile(artifactFilePath); - } - return BbPromise.resolve(); - }); + }, { concurrency: 3 } + ); }, }; diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js index 7af45d186..9cd72e81e 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js @@ -230,23 +230,57 @@ describe('uploadArtifacts', () => { }); }); - describe('#uploadFunctions()', () => { + describe.only('#uploadFunctions()', () => { + beforeEach(() => { + sinon.stub(fs, 'statSync').returns({ size: 1024 }); + }); + + afterEach(() => { + fs.statSync.restore(); + }); + it('should upload the service artifact file to the S3 bucket', () => { awsDeploy.serverless.config.servicePath = 'some/path'; awsDeploy.serverless.service.service = 'new-service'; - sinon.stub(fs, 'statSync').returns({ size: 0 }); - const uploadZipFileStub = sinon .stub(awsDeploy, 'uploadZipFile').resolves(); return awsDeploy.uploadFunctions().then(() => { expect(uploadZipFileStub.calledOnce).to.be.equal(true); - fs.statSync.restore(); awsDeploy.uploadZipFile.restore(); }); }); + // it.only('should upload a single .zip file to the S3 bucket', () => { + // awsDeploy.serverless.config.servicePath = 'some/path'; + // awsDeploy.serverless.service.service = 'new-service'; + // awsDeploy.serverless.service.functions = { + // first: { + // handler: 'first-handler', + // }, + // second: { + // handler: 'second-handler', + // }, + + // }; + + // sinon.stub(fs, 'statSync').returns({ size: 0 }); + + // const uploadZipFileStub = sinon + // .stub(awsDeploy, 'uploadZipFile').resolves(); + + // return awsDeploy.uploadFunctions().then(() => { + // expect(uploadZipFileStub.calledOnce).to.be.equal(true); + // console.log(`uploadZipFileStub.args ${JSON.stringify(uploadZipFileStub.args, null, 2)}`) + // expect(uploadZipFileStub.args[0][0]) + // .to.be.equal(awsDeploy.serverless.service.functions.first.package.artifact); + // expect(uploadZipFileStub.args[1][0]) + // .to.be.equal(awsDeploy.serverless.service.functions.second.package.artifact); + // awsDeploy.uploadZipFile.restore(); + // }); + // }); + it('should upload the function .zip files to the S3 bucket', () => { awsDeploy.serverless.service.package.individually = true; awsDeploy.serverless.service.functions = { @@ -292,7 +326,6 @@ describe('uploadArtifacts', () => { const uploadZipFileStub = sinon .stub(awsDeploy, 'uploadZipFile').resolves(); - sinon.stub(fs, 'statSync').returns({ size: 1024 }); return awsDeploy.uploadFunctions().then(() => { expect(uploadZipFileStub.calledTwice).to.be.equal(true); @@ -301,7 +334,6 @@ describe('uploadArtifacts', () => { expect(uploadZipFileStub.args[1][0]) .to.be.equal(awsDeploy.serverless.service.package.artifact); awsDeploy.uploadZipFile.restore(); - fs.statSync.restore(); }); }); @@ -309,7 +341,6 @@ describe('uploadArtifacts', () => { awsDeploy.serverless.config.servicePath = 'some/path'; awsDeploy.serverless.service.service = 'new-service'; - sinon.stub(fs, 'statSync').returns({ size: 1024 }); sinon.stub(awsDeploy, 'uploadZipFile').resolves(); sinon.spy(awsDeploy.serverless.cli, 'log'); @@ -317,7 +348,6 @@ describe('uploadArtifacts', () => { const expected = 'Uploading service .zip file to S3 (1 KB)...'; expect(awsDeploy.serverless.cli.log.calledWithExactly(expected)).to.be.equal(true); - fs.statSync.restore(); awsDeploy.uploadZipFile.restore(); }); }); From 3e7f736abcb02ec5effc6f0938c194c82c3b0354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Mon, 22 Jan 2018 14:58:34 -0200 Subject: [PATCH 02/10] Fix concurrency upload --- lib/plugins/aws/deploy/lib/uploadArtifacts.js | 5 +-- .../aws/deploy/lib/uploadArtifacts.test.js | 45 +++++++++---------- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.js index 2c9baa953..e2ce3e4d1 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.js @@ -91,7 +91,7 @@ module.exports = { functionObject.package = functionObject.package || {}; const artifact = functionObject.package.artifact || this.serverless.service.package.artifact; - console.log(`artifactFilePath ${JSON.stringify(artifact, null, 2)}`) + if (!artifact || (this.serverless.service.artifact && !functionObject.package.artifact)) { if (this.serverless.service.package.individually || functionObject.package.individually) { @@ -100,12 +100,11 @@ module.exports = { } return this.provider.naming.getServiceArtifactName(); } + return artifact; }) ); - console.log(`artifactFileNames ${JSON.stringify(artifactFilePaths, null, 2)}`) - return BbPromise.map(artifactFilePaths, (artifactFilePath) => { const stats = fs.statSync(artifactFilePath); this.serverless.cli.log(`Uploading service .zip file to S3 (${filesize(stats.size)})...`); diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js index 9cd72e81e..9f50614a0 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js @@ -252,34 +252,29 @@ describe('uploadArtifacts', () => { }); }); - // it.only('should upload a single .zip file to the S3 bucket', () => { - // awsDeploy.serverless.config.servicePath = 'some/path'; - // awsDeploy.serverless.service.service = 'new-service'; - // awsDeploy.serverless.service.functions = { - // first: { - // handler: 'first-handler', - // }, - // second: { - // handler: 'second-handler', - // }, + it('should upload a single .zip file to the S3 bucket', () => { + awsDeploy.serverless.config.servicePath = 'some/path'; + awsDeploy.serverless.service.service = 'new-service'; + awsDeploy.serverless.service.functions = { + first: { + handler: 'first-handler', + }, + second: { + handler: 'second-handler', + }, - // }; + }; - // sinon.stub(fs, 'statSync').returns({ size: 0 }); + const uploadZipFileStub = sinon + .stub(awsDeploy, 'uploadZipFile').resolves(); - // const uploadZipFileStub = sinon - // .stub(awsDeploy, 'uploadZipFile').resolves(); - - // return awsDeploy.uploadFunctions().then(() => { - // expect(uploadZipFileStub.calledOnce).to.be.equal(true); - // console.log(`uploadZipFileStub.args ${JSON.stringify(uploadZipFileStub.args, null, 2)}`) - // expect(uploadZipFileStub.args[0][0]) - // .to.be.equal(awsDeploy.serverless.service.functions.first.package.artifact); - // expect(uploadZipFileStub.args[1][0]) - // .to.be.equal(awsDeploy.serverless.service.functions.second.package.artifact); - // awsDeploy.uploadZipFile.restore(); - // }); - // }); + return awsDeploy.uploadFunctions().then(() => { + expect(uploadZipFileStub.calledOnce).to.be.equal(true); + expect(uploadZipFileStub.args[0][0]) + .to.be.equal(`${awsDeploy.serverless.service.service}.zip`); + awsDeploy.uploadZipFile.restore(); + }); + }); it('should upload the function .zip files to the S3 bucket', () => { awsDeploy.serverless.service.package.individually = true; From d059a3c448cc7d3fdf8f4477a064fc4754cac613 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Mon, 22 Jan 2018 14:59:35 -0200 Subject: [PATCH 03/10] Improve spec description --- lib/plugins/aws/deploy/lib/uploadArtifacts.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js index 9f50614a0..5d25bc71c 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js @@ -252,7 +252,7 @@ describe('uploadArtifacts', () => { }); }); - it('should upload a single .zip file to the S3 bucket', () => { + it('should upload a single .zip file to the S3 bucket when not packaging individually', () => { awsDeploy.serverless.config.servicePath = 'some/path'; awsDeploy.serverless.service.service = 'new-service'; awsDeploy.serverless.service.functions = { From 29e13014db6f489f00d44992bf6375e2342da410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Mon, 22 Jan 2018 15:32:02 -0200 Subject: [PATCH 04/10] artifactFilePath makes more sense --- lib/plugins/aws/deploy/lib/uploadArtifacts.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.js index e2ce3e4d1..1d2a59862 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.js @@ -89,10 +89,10 @@ module.exports = { const functionArtifactFileName = this.provider.naming.getFunctionArtifactName(name); const functionObject = this.serverless.service.getFunction(name); functionObject.package = functionObject.package || {}; - const artifact = functionObject.package.artifact || + const artifactFilePath = functionObject.package.artifact || this.serverless.service.package.artifact; - if (!artifact || + if (!artifactFilePath || (this.serverless.service.artifact && !functionObject.package.artifact)) { if (this.serverless.service.package.individually || functionObject.package.individually) { const artifactFileName = functionArtifactFileName; @@ -101,7 +101,7 @@ module.exports = { return this.provider.naming.getServiceArtifactName(); } - return artifact; + return artifactFilePath; }) ); From cc0fee96b7909a0ffc1edfa6dc8be2b2bf10aba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Mon, 22 Jan 2018 22:15:57 -0200 Subject: [PATCH 05/10] Adding spec to test that duplicates uploads --- .../aws/deploy/lib/uploadArtifacts.test.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js index 5d25bc71c..8c3f0eb70 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js @@ -230,7 +230,7 @@ describe('uploadArtifacts', () => { }); }); - describe.only('#uploadFunctions()', () => { + describe('#uploadFunctions()', () => { beforeEach(() => { sinon.stub(fs, 'statSync').returns({ size: 1024 }); }); @@ -253,16 +253,17 @@ describe('uploadArtifacts', () => { }); it('should upload a single .zip file to the S3 bucket when not packaging individually', () => { - awsDeploy.serverless.config.servicePath = 'some/path'; - awsDeploy.serverless.service.service = 'new-service'; awsDeploy.serverless.service.functions = { first: { - handler: 'first-handler', + package: { + artifact: 'artifact.zip', + }, }, second: { - handler: 'second-handler', + package: { + artifact: 'artifact.zip', + }, }, - }; const uploadZipFileStub = sinon @@ -270,8 +271,7 @@ describe('uploadArtifacts', () => { return awsDeploy.uploadFunctions().then(() => { expect(uploadZipFileStub.calledOnce).to.be.equal(true); - expect(uploadZipFileStub.args[0][0]) - .to.be.equal(`${awsDeploy.serverless.service.service}.zip`); + expect(uploadZipFileStub.args[0][0]).to.be.equal('artifact.zip'); awsDeploy.uploadZipFile.restore(); }); }); From ede4824c97329af0bc5607960453bebb9d1d1bff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Mon, 22 Jan 2018 22:30:53 -0200 Subject: [PATCH 06/10] Improve tests --- .../aws/deploy/lib/uploadArtifacts.test.js | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js index 8c3f0eb70..b743a0db1 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js @@ -231,24 +231,24 @@ describe('uploadArtifacts', () => { }); describe('#uploadFunctions()', () => { + let uploadZipFileStub; + beforeEach(() => { sinon.stub(fs, 'statSync').returns({ size: 1024 }); + uploadZipFileStub = sinon.stub(awsDeploy, 'uploadZipFile').resolves(); }); afterEach(() => { fs.statSync.restore(); + uploadZipFileStub.restore(); }); it('should upload the service artifact file to the S3 bucket', () => { awsDeploy.serverless.config.servicePath = 'some/path'; awsDeploy.serverless.service.service = 'new-service'; - const uploadZipFileStub = sinon - .stub(awsDeploy, 'uploadZipFile').resolves(); - return awsDeploy.uploadFunctions().then(() => { expect(uploadZipFileStub.calledOnce).to.be.equal(true); - awsDeploy.uploadZipFile.restore(); }); }); @@ -266,13 +266,9 @@ describe('uploadArtifacts', () => { }, }; - const uploadZipFileStub = sinon - .stub(awsDeploy, 'uploadZipFile').resolves(); - return awsDeploy.uploadFunctions().then(() => { expect(uploadZipFileStub.calledOnce).to.be.equal(true); expect(uploadZipFileStub.args[0][0]).to.be.equal('artifact.zip'); - awsDeploy.uploadZipFile.restore(); }); }); @@ -291,16 +287,12 @@ describe('uploadArtifacts', () => { }, }; - const uploadZipFileStub = sinon - .stub(awsDeploy, 'uploadZipFile').resolves(); - return awsDeploy.uploadFunctions().then(() => { expect(uploadZipFileStub.calledTwice).to.be.equal(true); expect(uploadZipFileStub.args[0][0]) .to.be.equal(awsDeploy.serverless.service.functions.first.package.artifact); expect(uploadZipFileStub.args[1][0]) .to.be.equal(awsDeploy.serverless.service.functions.second.package.artifact); - awsDeploy.uploadZipFile.restore(); }); }); @@ -319,16 +311,12 @@ describe('uploadArtifacts', () => { }, }; - const uploadZipFileStub = sinon - .stub(awsDeploy, 'uploadZipFile').resolves(); - return awsDeploy.uploadFunctions().then(() => { expect(uploadZipFileStub.calledTwice).to.be.equal(true); expect(uploadZipFileStub.args[0][0]) .to.be.equal(awsDeploy.serverless.service.functions.first.package.artifact); expect(uploadZipFileStub.args[1][0]) .to.be.equal(awsDeploy.serverless.service.package.artifact); - awsDeploy.uploadZipFile.restore(); }); }); @@ -336,14 +324,11 @@ describe('uploadArtifacts', () => { awsDeploy.serverless.config.servicePath = 'some/path'; awsDeploy.serverless.service.service = 'new-service'; - sinon.stub(awsDeploy, 'uploadZipFile').resolves(); sinon.spy(awsDeploy.serverless.cli, 'log'); return awsDeploy.uploadFunctions().then(() => { const expected = 'Uploading service .zip file to S3 (1 KB)...'; expect(awsDeploy.serverless.cli.log.calledWithExactly(expected)).to.be.equal(true); - - awsDeploy.uploadZipFile.restore(); }); }); }); From 0fe3efefe9bd2554a4ac2b4514ddca582476f06a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Tue, 23 Jan 2018 08:55:55 -0200 Subject: [PATCH 07/10] Adding verification to expected file name --- lib/plugins/aws/deploy/lib/uploadArtifacts.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js index b881a1298..306610af5 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.test.js @@ -243,6 +243,7 @@ describe('uploadArtifacts', () => { return awsDeploy.uploadFunctions().then(() => { expect(uploadZipFileStub.calledOnce).to.be.equal(true); + expect(uploadZipFileStub.args[0][0]).to.be.equal('new-service.zip'); }); }); From 3aeb1c533a130a35158c477a85f0555a91ccdf35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Wed, 28 Feb 2018 09:56:43 -0300 Subject: [PATCH 08/10] Retry travis From 5ed408aedc558999c9e21229cdebec4fd0fd20c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Wed, 28 Feb 2018 10:28:49 -0300 Subject: [PATCH 09/10] Retry travis From 005b6bdcdde1c788c894773e5ca362c69baadfc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Guimar=C3=A3es?= Date: Mon, 19 Mar 2018 10:27:11 -0300 Subject: [PATCH 10/10] Adding NUM_CONCURRENT_UPLOADS constant --- lib/plugins/aws/deploy/lib/uploadArtifacts.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/plugins/aws/deploy/lib/uploadArtifacts.js b/lib/plugins/aws/deploy/lib/uploadArtifacts.js index 20fc8e42c..8d6c94556 100644 --- a/lib/plugins/aws/deploy/lib/uploadArtifacts.js +++ b/lib/plugins/aws/deploy/lib/uploadArtifacts.js @@ -10,6 +10,8 @@ const BbPromise = require('bluebird'); const filesize = require('filesize'); const normalizeFiles = require('../../lib/normalizeFiles'); +const NUM_CONCURRENT_UPLOADS = 3; + module.exports = { uploadArtifacts() { return BbPromise.bind(this) @@ -105,7 +107,7 @@ module.exports = { const stats = fs.statSync(artifactFilePath); this.serverless.cli.log(`Uploading service .zip file to S3 (${filesize(stats.size)})...`); return this.uploadZipFile(artifactFilePath); - }, { concurrency: 3 } + }, { concurrency: NUM_CONCURRENT_UPLOADS } ); }, };