From 77ec30963232ed3b0313f26ccb730aa16169b7a0 Mon Sep 17 00:00:00 2001 From: Cristian Cavalli Date: Thu, 6 Apr 2017 11:14:28 -0700 Subject: [PATCH 1/6] Update env.js Update env.js with errors system-test required fields: `apiKey` `projectNumber` --- system-test/env.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/system-test/env.js b/system-test/env.js index 2a21b366adc..9902499963c 100644 --- a/system-test/env.js +++ b/system-test/env.js @@ -18,5 +18,7 @@ module.exports = { projectId: process.env.GCLOUD_TESTS_PROJECT_ID, - keyFilename: process.env.GCLOUD_TESTS_KEY + keyFilename: process.env.GCLOUD_TESTS_KEY, + apiKey: process.env.GCLOUD_TESTS_API_KEY, + projectNumber: process.env.GCLOUD_TESTS_PROJECT_NUMBER }; From b5430729e9848963d157e6241c8d13117fc61cba Mon Sep 17 00:00:00 2001 From: Cristian Cavalli Date: Thu, 6 Apr 2017 12:13:21 -0700 Subject: [PATCH 2/6] Update testAuthClient.js Point env checks and sourcing to env file. --- .../system-test/testAuthClient.js | 73 +++++++++---------- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/packages/error-reporting/system-test/testAuthClient.js b/packages/error-reporting/system-test/testAuthClient.js index c0b02ab1ba4..ed593750962 100644 --- a/packages/error-reporting/system-test/testAuthClient.js +++ b/packages/error-reporting/system-test/testAuthClient.js @@ -28,21 +28,41 @@ var isEmpty = is.empty; var forEach = require('lodash.foreach'); var assign = require('lodash.assign'); const ERR_TOKEN = '_@google_STACKDRIVER_INTEGRATION_TEST_ERROR__'; - +const env = (function(injectedEnv) { + class InstancedEnv { + constructor() { + assign(this, injectedEnv); + this._assignNodeEnv(); + } + _assignNodeEnv() { + this.NODE_ENV = 'production'; + } + sterilize() { + forEach(Object.keys(injectedEnv).concat(['NODE_ENV']), (v, k) => delete this[k]); + return this; + } + restore() { + assign(this, injectedEnv); + this._assignNodeEnv(); + return this; + } + } + return new InstancedEnv(); +}(require('../../../system-test/env.js'))); describe('Behvaiour acceptance testing', function() { before(function() { // Before starting the suite make sure we have the proper resources - if (!isString(process.env.GCLOUD_PROJECT)) { + if (!isString(env.projectId)) { throw new Error( - 'The gcloud project id (GCLOUD_PROJECT) was not set in the env'); - } else if (!isString(process.env.STUBBED_API_KEY)) { + 'The gcloud project id (projectId) was not set in the env'); + } else if (!isString(env.apiKey)) { throw new Error( - 'The api key (STUBBED_API_KEY) was not set as an env variable'); - } else if (!isString(process.env.STUBBED_PROJECT_NUM)) { + 'The api key (apiKey) was not set as an env variable'); + } else if (!isString(env.projectNumber)) { throw new Error( - 'The project number (STUBBED_PROJECT_NUM) was not set in the env'); - } else if (process.env.NODE_ENV !== 'production') { + 'The project number (projectNumber) was not set in the env'); + } else if (env.NODE_ENV !== 'production') { throw new Error( 'The NODE_ENV is not set to production as an env variable. Please ' + 'set NODE_ENV to production'); @@ -57,7 +77,7 @@ describe('Behvaiour acceptance testing', function() { beforeEach(function() { fakeService = nock( 'https://clouderrorreporting.googleapis.com/v1beta1/projects/' + - process.env.GCLOUD_PROJECT + env.projectId ).persist().post('/events:report'); logger = createLogger({logLevel: 5}); client = new RequestHandler( @@ -99,7 +119,7 @@ describe('Behvaiour acceptance testing', function() { describe('Using an API key', function() { it('Should provide the key as a query string on outgoing requests', function(done) { - var key = process.env.STUBBED_API_KEY; + var key = env.apiKey; var client = new RequestHandler(new Configuration( {key: key, ignoreEnvironmentCheck: true}, createLogger({logLevel: 5}))); @@ -126,8 +146,8 @@ describe('Behvaiour acceptance testing', function() { var sampleError = new Error(ERR_TOKEN); var errorMessage = new ErrorMessage().setMessage(sampleError.stack); var oldEnv = { - GCLOUD_PROJECT: process.env.GCLOUD_PROJECT, - STUBBED_PROJECT_NUM: process.env.STUBBED_PROJECT_NUM, + GCLOUD_PROJECT: env.projectId, + STUBBED_PROJECT_NUM: env.projectNumber, NODE_ENV: process.env.NODE_ENV }; function sterilizeEnv() { @@ -167,7 +187,7 @@ describe('Behvaiour acceptance testing', function() { var cfg, logger; before(function() { sterilizeEnv(); - process.env.GCLOUD_PROJECT = oldEnv.GCLOUD_PROJECT; + env.projectId = oldEnv.GCLOUD_PROJECT; logger = createLogger({logLevel: 5}); cfg = new Configuration({ignoreEnvironmentCheck: true}, logger); }); @@ -265,33 +285,6 @@ describe('Behvaiour acceptance testing', function() { }); }); }); - // describe('An invalid env configuration', function() { - // var ERROR_STRING = [ - // 'Unable to find the project Id for communication with the', - // 'Stackdriver Error Reporting service. This app will be unable to', - // 'send errors to the reporting service unless a valid project Id', - // 'is supplied via runtime configuration or the GCLOUD_PROJECT', - // 'environmental variable.' - // ].join(' '); - // var logger, client; - // before(function() { - // delete process.env.GCLOUD_PROJECT; - // logger = createLogger({logLevel: 5}); - // client = new RequestHandler(new Configuration( - // {ignoreEnvironmentCheck: true}, logger), logger); - // }); - // after(function() { - // process.env.GCLOUD_PROJECT = oldEnv.GCLOUD_PROJECT; - // }); - // it('Should callback with an error', function(done) { - // client.sendError(errorMessage, function(err, response, body) { - // assert(err instanceof Error); - // assert.strictEqual(err.message, ERROR_STRING); - // assert.strictEqual(response, null); - // done(); - // }); - // }); - // }); }); describe('Success behaviour', function() { var er = new Error(ERR_TOKEN); From 8b8844abcb96ae73a742140ad168c4102aeb838d Mon Sep 17 00:00:00 2001 From: Cristian Cavalli Date: Thu, 6 Apr 2017 13:57:33 -0700 Subject: [PATCH 3/6] update system tests --- packages/error-reporting/package.json | 1 + .../system-test/testAuthClient.js | 165 +++++++++++------- 2 files changed, 99 insertions(+), 67 deletions(-) diff --git a/packages/error-reporting/package.json b/packages/error-reporting/package.json index 7bdcd643a36..d067e49be1a 100644 --- a/packages/error-reporting/package.json +++ b/packages/error-reporting/package.json @@ -24,6 +24,7 @@ "lodash.merge": "^4.6.0", "lodash.omit": "^4.5.0", "lodash.omitby": "^4.6.0", + "lodash.pick": "^4.4.0", "lodash.random": "^3.2.0", "lodash.without": "^4.4.0", "mocha": "^3.2.0", diff --git a/packages/error-reporting/system-test/testAuthClient.js b/packages/error-reporting/system-test/testAuthClient.js index ed593750962..7c66e209ad7 100644 --- a/packages/error-reporting/system-test/testAuthClient.js +++ b/packages/error-reporting/system-test/testAuthClient.js @@ -27,54 +27,89 @@ var isString = is.string; var isEmpty = is.empty; var forEach = require('lodash.foreach'); var assign = require('lodash.assign'); +var pick = require('lodash.pick'); +var omitBy = require('lodash.omitby'); const ERR_TOKEN = '_@google_STACKDRIVER_INTEGRATION_TEST_ERROR__'; const env = (function(injectedEnv) { + const envKeys = ['GOOGLE_APPLICATION_CREDENTIALS', 'GCLOUD_PROJECT', + 'NODE_ENV']; class InstancedEnv { constructor() { assign(this, injectedEnv); - this._assignNodeEnv(); + this._originalEnv = this._captureProcessProperties(); } - _assignNodeEnv() { - this.NODE_ENV = 'production'; + _captureProcessProperties() { + return omitBy(pick(process.env, envKeys), value => !isString(value)); } - sterilize() { - forEach(Object.keys(injectedEnv).concat(['NODE_ENV']), (v, k) => delete this[k]); + sterilizeProcess() { + forEach(envKeys, (v, k) => delete process.env[k]); return this; } - restore() { - assign(this, injectedEnv); - this._assignNodeEnv(); + setProjectId() { + assign(process.env, { + GCLOUD_PROJECT: injectedEnv.projectId + }); + return this; + } + setProjectNumber() { + assign(process.env, { + GCLOUD_PROJECT: injectedEnv.projectNumber + }); + return this; + } + setKeyFilename() { + assign(process.env, { + GOOGLE_APPLICATION_CREDENTIALS: injectedEnv.keyFilename + }); + return this; + } + setProduction() { + assign(process.env, { + NODE_ENV: 'production' + }); + return this; + } + restoreProcessToOriginalState() { + assign(process.env, this._originalEnv); return this; } + injected () { + return assign({}, injectedEnv); + } } return new InstancedEnv(); }(require('../../../system-test/env.js'))); +const SHOULD_RUN = (function () { + if (!isString(env.injected().projectId)) { + return new Error('The project id (projectId) was not set in the env'); + } else if (!isString(env.injected().apiKey)) { + return new Error('The api key (apiKey) was not set as an env variable'); + } else if (!isString(env.injected().projectNumber)) { + return new Error( + 'The project number (projectNumber) was not set in the env'); + } else if (!isString(env.injected().keyFilename)) { + return new Error( + 'The key filename (keyFilename) was not set in the env'); + } + return true; +}()); +const TEST_RUNNER = (function () { + if (SHOULD_RUN instanceof Error) { + console.log('Skipping error-reporting system tests:'); + console.log(' '+SHOULD_RUN.message); + return describe.skip; + } + return describe; +}()); -describe('Behvaiour acceptance testing', function() { - before(function() { - // Before starting the suite make sure we have the proper resources - if (!isString(env.projectId)) { - throw new Error( - 'The gcloud project id (projectId) was not set in the env'); - } else if (!isString(env.apiKey)) { - throw new Error( - 'The api key (apiKey) was not set as an env variable'); - } else if (!isString(env.projectNumber)) { - throw new Error( - 'The project number (projectNumber) was not set in the env'); - } else if (env.NODE_ENV !== 'production') { - throw new Error( - 'The NODE_ENV is not set to production as an env variable. Please ' + - 'set NODE_ENV to production'); - } - // In case we are running after unit mocks which were not destroyed properly - nock.cleanAll(); - }); +(TEST_RUNNER)('Errors system tests', function() { describe('Request/Response lifecycle mocking', function() { var sampleError = new Error(ERR_TOKEN); var errorMessage = new ErrorMessage().setMessage(sampleError); var fakeService, client, logger; + before(() => env.sterilizeProcess()); beforeEach(function() { + env.setProjectId().setKeyFilename().setProduction(); fakeService = nock( 'https://clouderrorreporting.googleapis.com/v1beta1/projects/' + env.projectId @@ -84,8 +119,10 @@ describe('Behvaiour acceptance testing', function() { new Configuration({ignoreEnvironmentCheck: true}, logger), logger); }); afterEach(function() { + env.sterilizeProcess(); nock.cleanAll(); }); + after(() => env.restoreProcessToOriginalState()); describe('Receiving non-retryable errors', function() { it('Should fail', function(done) { this.timeout(5000); @@ -119,6 +156,7 @@ describe('Behvaiour acceptance testing', function() { describe('Using an API key', function() { it('Should provide the key as a query string on outgoing requests', function(done) { + env.sterilizeProcess().setProjectId().setProduction(); var key = env.apiKey; var client = new RequestHandler(new Configuration( {key: key, ignoreEnvironmentCheck: true}, @@ -145,30 +183,30 @@ describe('Behvaiour acceptance testing', function() { describe('System-live integration testing', function() { var sampleError = new Error(ERR_TOKEN); var errorMessage = new ErrorMessage().setMessage(sampleError.stack); - var oldEnv = { - GCLOUD_PROJECT: env.projectId, - STUBBED_PROJECT_NUM: env.projectNumber, - NODE_ENV: process.env.NODE_ENV - }; - function sterilizeEnv() { - forEach(oldEnv, function(val, key) { - delete process.env[key]; - }); - } - function restoreEnv() { - assign(process.env, oldEnv); - } + // var oldEnv = { + // GCLOUD_PROJECT: env.projectId, + // STUBBED_PROJECT_NUM: env.projectNumber, + // NODE_ENV: process.env.NODE_ENV + // }; + // function sterilizeEnv() { + // forEach(oldEnv, function(val, key) { + // delete process.env[key]; + // }); + // } + // function restoreEnv() { + // assign(process.env, oldEnv); + // } describe('Client creation', function() { describe('Using only project id', function() { describe('As a runtime argument', function() { var cfg, logger; before(function() { - sterilizeEnv(); + env.sterilizeProcess().setKeyFilename(); logger = createLogger({logLevel: 5}); - cfg = new Configuration({projectId: oldEnv.GCLOUD_PROJECT, + cfg = new Configuration({projectId: env.injected().projectId, ignoreEnvironmentCheck: true}, logger); }); - after(restoreEnv); + after(() => env.sterilizeProcess()); it('Should not throw on initialization', function(done) { this.timeout(10000); assert.doesNotThrow(function() { @@ -186,12 +224,11 @@ describe('Behvaiour acceptance testing', function() { describe('As an env variable', function() { var cfg, logger; before(function() { - sterilizeEnv(); - env.projectId = oldEnv.GCLOUD_PROJECT; + env.sterilizeProcess().setProjectId().setKeyFilename(); logger = createLogger({logLevel: 5}); cfg = new Configuration({ignoreEnvironmentCheck: true}, logger); }); - after(restoreEnv); + after(() => env.sterilizeProcess()); it('Should not throw on initialization', function(done) { this.timeout(10000); assert.doesNotThrow(function() { @@ -211,14 +248,14 @@ describe('Behvaiour acceptance testing', function() { describe('As a runtime argument', function() { var cfg, logger; before(function() { - sterilizeEnv(); + env.sterilizeProcess().setKeyFilename(); logger = createLogger({logLevel: 5}); cfg = new Configuration({ - projectId: parseInt(oldEnv.STUBBED_PROJECT_NUM), + projectId: parseInt(env.injected().projectNumber), ignoreEnvironmentCheck: true }, logger); }); - after(restoreEnv); + after(() => env.sterilizeProcess()); it('Should not throw on initialization', function(done) { this.timeout(10000); assert.doesNotThrow(function() { @@ -236,12 +273,11 @@ describe('Behvaiour acceptance testing', function() { describe('As an env variable', function() { var cfg, logger; before(function() { - sterilizeEnv(); - process.env.GCLOUD_PROJECT = oldEnv.STUBBED_PROJECT_NUM; + env.sterilizeProcess().setKeyFilename().setProjectNumber(); logger = createLogger({logLevel: 5}); cfg = new Configuration({ignoreEnvironmentCheck: true}, logger); }); - after(restoreEnv); + after(() => env.sterilizeProcess()); it('Should not throw on initialization', function(done) { this.timeout(10000); assert.doesNotThrow(function() { @@ -268,14 +304,13 @@ describe('Behvaiour acceptance testing', function() { ].join(' '); var logger, client; before(function() { - delete process.env.NODE_ENV; + env.sterilizeProcess().setKeyFilename().setProjectId(); + process.env.NODE_ENV = 'null'; logger = createLogger({logLevel: 5}); client = new RequestHandler(new Configuration(undefined, logger), logger); }); - after(function() { - process.env.NODE_ENV = oldEnv.NODE_ENV; - }); + after(() => env.sterilizeProcess()); it('Should callback with an error', function(done) { client.sendError({}, function(err, response, body) { assert(err instanceof Error); @@ -292,15 +327,15 @@ describe('Behvaiour acceptance testing', function() { describe('Given a valid project id', function() { var logger, client, cfg; before(function() { - sterilizeEnv(); + env.sterilizeProcess(); logger = createLogger({logLevel: 5}); cfg = new Configuration({ - projectId: oldEnv.GCLOUD_PROJECT, + projectId: env.injected().projectId, ignoreEnvironmentCheck: true }, logger); client = new RequestHandler(cfg, logger); }); - after(restoreEnv); + after(() => env.sterilizeProcess()); it('Should succeed in its request', function(done) { client.sendError(em, function(err, response, body) { assert.strictEqual(err, null); @@ -314,19 +349,15 @@ describe('Behvaiour acceptance testing', function() { describe('Given a valid project number', function() { var logger, client, cfg; before(function() { - forEach(oldEnv, function(val, key) { - delete process.env[key]; - }); + env.sterilizeProcess(); logger = createLogger({logLevel: 5}); cfg = new Configuration({ - projectId: parseInt(oldEnv.STUBBED_PROJECT_NUM), + projectId: parseInt(env.injected().projectNumber), ignoreEnvironmentCheck: true }, logger); client = new RequestHandler(cfg, logger); }); - after(function() { - assign(process.env, oldEnv); - }); + after(() => env.sterilizeProcess()); it('Should succeed in its request', function(done) { client.sendError(em, function(err, response, body) { assert.strictEqual(err, null); From ef0635c6e6bf59cb1927e54b07ba294bdbc9cefb Mon Sep 17 00:00:00 2001 From: Cristian Cavalli Date: Thu, 6 Apr 2017 14:01:24 -0700 Subject: [PATCH 4/6] fix linting --- .../system-test/testAuthClient.js | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/packages/error-reporting/system-test/testAuthClient.js b/packages/error-reporting/system-test/testAuthClient.js index 7c66e209ad7..ea733123e42 100644 --- a/packages/error-reporting/system-test/testAuthClient.js +++ b/packages/error-reporting/system-test/testAuthClient.js @@ -73,30 +73,33 @@ const env = (function(injectedEnv) { assign(process.env, this._originalEnv); return this; } - injected () { + injected() { return assign({}, injectedEnv); } } return new InstancedEnv(); }(require('../../../system-test/env.js'))); -const SHOULD_RUN = (function () { +const SHOULD_RUN = (function() { if (!isString(env.injected().projectId)) { return new Error('The project id (projectId) was not set in the env'); - } else if (!isString(env.injected().apiKey)) { + } + if (!isString(env.injected().apiKey)) { return new Error('The api key (apiKey) was not set as an env variable'); - } else if (!isString(env.injected().projectNumber)) { + } + if (!isString(env.injected().projectNumber)) { return new Error( 'The project number (projectNumber) was not set in the env'); - } else if (!isString(env.injected().keyFilename)) { + } + if (!isString(env.injected().keyFilename)) { return new Error( 'The key filename (keyFilename) was not set in the env'); } return true; }()); -const TEST_RUNNER = (function () { +const TEST_RUNNER = (function() { if (SHOULD_RUN instanceof Error) { console.log('Skipping error-reporting system tests:'); - console.log(' '+SHOULD_RUN.message); + console.log(' ' + SHOULD_RUN.message); return describe.skip; } return describe; @@ -183,19 +186,6 @@ const TEST_RUNNER = (function () { describe('System-live integration testing', function() { var sampleError = new Error(ERR_TOKEN); var errorMessage = new ErrorMessage().setMessage(sampleError.stack); - // var oldEnv = { - // GCLOUD_PROJECT: env.projectId, - // STUBBED_PROJECT_NUM: env.projectNumber, - // NODE_ENV: process.env.NODE_ENV - // }; - // function sterilizeEnv() { - // forEach(oldEnv, function(val, key) { - // delete process.env[key]; - // }); - // } - // function restoreEnv() { - // assign(process.env, oldEnv); - // } describe('Client creation', function() { describe('Using only project id', function() { describe('As a runtime argument', function() { From c292c604b4b1641fe8d16a06a271280190802213 Mon Sep 17 00:00:00 2001 From: Cristian Cavalli Date: Thu, 13 Apr 2017 13:36:07 -0700 Subject: [PATCH 5/6] update CONTRIBUTING.md with new env var info --- .github/CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 94eafd56623..5917e249161 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -23,8 +23,9 @@ $ npm test To run the system tests, first create and configure a project in the Google Developers Console following the [instructions on how to run google-cloud-node][elsewhere]. After that, set the following environment variables: - **GCLOUD_TESTS_PROJECT_ID**: Developers Console project's ID (e.g. bamboo-shift-455) +- ***GCLOUD_TESTS_PROJECT_NUMBER*** (*optional*): Developers Console project number (e.g. 1046198160504). Required to run system tests for error reporting. - **GCLOUD_TESTS_KEY**: The path to the JSON key file. -- ***GCLOUD_TESTS_API_KEY*** (*optional*): An API key that can be used to test the Translate API. +- ***GCLOUD_TESTS_API_KEY*** (*optional*): An API key that can be used to test the Translate API and is required to run system-test for error reporting. - ***GCLOUD_TESTS_DNS_DOMAIN*** (*optional*): A domain you own managed by Cloud DNS (expected format: `'gcloud-node.com.'`). Install the [gcloud command-line tool][gcloudcli] to your machine and use it to create the indexes used in the datastore system tests with indexes found in `system-test/data/index/yaml`: From 04a249b93b51172135e1edcce25df46b5ba57f6f Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Thu, 13 Apr 2017 16:42:30 -0400 Subject: [PATCH 6/6] Update CONTRIBUTING.md --- .github/CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 5917e249161..b0369811f87 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -23,9 +23,9 @@ $ npm test To run the system tests, first create and configure a project in the Google Developers Console following the [instructions on how to run google-cloud-node][elsewhere]. After that, set the following environment variables: - **GCLOUD_TESTS_PROJECT_ID**: Developers Console project's ID (e.g. bamboo-shift-455) -- ***GCLOUD_TESTS_PROJECT_NUMBER*** (*optional*): Developers Console project number (e.g. 1046198160504). Required to run system tests for error reporting. +- ***GCLOUD_TESTS_PROJECT_NUMBER*** (*optional*): Developers Console project number (e.g. 1046198160504). - **GCLOUD_TESTS_KEY**: The path to the JSON key file. -- ***GCLOUD_TESTS_API_KEY*** (*optional*): An API key that can be used to test the Translate API and is required to run system-test for error reporting. +- ***GCLOUD_TESTS_API_KEY*** (*optional*): An API key. - ***GCLOUD_TESTS_DNS_DOMAIN*** (*optional*): A domain you own managed by Cloud DNS (expected format: `'gcloud-node.com.'`). Install the [gcloud command-line tool][gcloudcli] to your machine and use it to create the indexes used in the datastore system tests with indexes found in `system-test/data/index/yaml`: