From bdc5170d43a6667e7c7d0175a2d2a8d485d79aaa Mon Sep 17 00:00:00 2001 From: horike37 Date: Sun, 23 Oct 2016 19:19:14 +0900 Subject: [PATCH 1/3] fixed issue 2500 --- lib/classes/Variables.js | 3 ++- lib/classes/Variables.test.js | 15 +++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/classes/Variables.js b/lib/classes/Variables.js index 0ff2b4b82..6f1fa57df 100644 --- a/lib/classes/Variables.js +++ b/lib/classes/Variables.js @@ -75,9 +75,10 @@ class Variables { populateVariable(propertyParam, matchedString, valueToPopulate) { let property = propertyParam; - if (typeof valueToPopulate === 'string') { property = replaceall(matchedString, valueToPopulate, property); + } else if (typeof valueToPopulate === 'number') { + property = replaceall(matchedString, String(valueToPopulate), property); } else { if (property !== matchedString) { const errorMessage = [ diff --git a/lib/classes/Variables.test.js b/lib/classes/Variables.test.js index bfef5ecd2..b60c9c59e 100644 --- a/lib/classes/Variables.test.js +++ b/lib/classes/Variables.test.js @@ -155,22 +155,21 @@ describe('Variables', () => { expect(newProperty).to.equal('my stage is dev'); }); - it('should populate non string variables', () => { + it('should populate number variables as sub string', () => { const serverless = new Serverless(); const valueToPopulate = 5; const matchedString = '${opt:number}'; - const property = '${opt:number}'; - + const property = 'your account number is ${opt:number}'; const newProperty = serverless.variables .populateVariable(property, matchedString, valueToPopulate); - expect(newProperty).to.equal(5); + expect(newProperty).to.equal('your account number is 5'); }); - it('should throw error if populating non string variable as sub string', () => { + it('should throw error if populating non string or non number variable as sub string', () => { const serverless = new Serverless(); - const valueToPopulate = 5; - const matchedString = '${opt:number}'; - const property = 'hello ${opt:number}'; + const valueToPopulate = {}; + const matchedString = '${opt:object}'; + const property = 'your account number is ${opt:object}'; expect(() => serverless.variables .populateVariable(property, matchedString, valueToPopulate)) .to.throw(Error); From 05b141ce681c3cdfd9996ca1e27fbcf91741e223 Mon Sep 17 00:00:00 2001 From: horike37 Date: Mon, 24 Oct 2016 22:32:48 +0900 Subject: [PATCH 2/3] Restore the test --- lib/classes/Variables.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/classes/Variables.test.js b/lib/classes/Variables.test.js index b60c9c59e..2d3e5572b 100644 --- a/lib/classes/Variables.test.js +++ b/lib/classes/Variables.test.js @@ -165,6 +165,17 @@ describe('Variables', () => { expect(newProperty).to.equal('your account number is 5'); }); + it('should populate non string variables', () => { + const serverless = new Serverless(); + const valueToPopulate = 5; + const matchedString = '${opt:number}'; + const property = '${opt:number}'; + + const newProperty = serverless.variables + .populateVariable(property, matchedString, valueToPopulate); + expect(newProperty).to.equal('5'); + }); + it('should throw error if populating non string or non number variable as sub string', () => { const serverless = new Serverless(); const valueToPopulate = {}; From 56ca36c4003956840fac4593f8c0c9c83076fc9d Mon Sep 17 00:00:00 2001 From: horike37 Date: Thu, 27 Oct 2016 01:24:16 +0900 Subject: [PATCH 3/3] fixed test that `should populate non string variables` --- lib/classes/Variables.js | 21 ++++++++++++--------- lib/classes/Variables.test.js | 3 ++- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/classes/Variables.js b/lib/classes/Variables.js index 6f1fa57df..22ff36e3c 100644 --- a/lib/classes/Variables.js +++ b/lib/classes/Variables.js @@ -77,17 +77,20 @@ class Variables { let property = propertyParam; if (typeof valueToPopulate === 'string') { property = replaceall(matchedString, valueToPopulate, property); - } else if (typeof valueToPopulate === 'number') { - property = replaceall(matchedString, String(valueToPopulate), property); } else { if (property !== matchedString) { - const errorMessage = [ - 'Trying to populate non string value into', - ` a string for variable ${matchedString}.`, - ' Please make sure the value of the property is a string.', - ].join(''); - throw new this.serverless.classes - .Error(errorMessage); + if (typeof valueToPopulate === 'number') { + property = replaceall(matchedString, String(valueToPopulate), property); + } else { + const errorMessage = [ + 'Trying to populate non string value into', + ` a string for variable ${matchedString}.`, + ' Please make sure the value of the property is a string.', + ].join(''); + throw new this.serverless.classes + .Error(errorMessage); + } + return property; } property = valueToPopulate; } diff --git a/lib/classes/Variables.test.js b/lib/classes/Variables.test.js index 2d3e5572b..ba6b1332c 100644 --- a/lib/classes/Variables.test.js +++ b/lib/classes/Variables.test.js @@ -160,6 +160,7 @@ describe('Variables', () => { const valueToPopulate = 5; const matchedString = '${opt:number}'; const property = 'your account number is ${opt:number}'; + const newProperty = serverless.variables .populateVariable(property, matchedString, valueToPopulate); expect(newProperty).to.equal('your account number is 5'); @@ -173,7 +174,7 @@ describe('Variables', () => { const newProperty = serverless.variables .populateVariable(property, matchedString, valueToPopulate); - expect(newProperty).to.equal('5'); + expect(newProperty).to.equal(5); }); it('should throw error if populating non string or non number variable as sub string', () => {