Adds tests for HTTP device sample #534

gguuss · 2017-12-12T02:25:41Z

Also adds configuration retrieval to http device sample.

codecov · 2017-12-13T19:11:06Z

Codecov Report

Merging #534 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #534   +/-   ##
=======================================
  Coverage   97.16%   97.16%           
=======================================
  Files          13       13           
  Lines         458      458           
=======================================
  Hits          445      445           
  Misses         13       13

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa2b04f...d118f5d. Read the comment docs.

ace-n

A few minor nits/changes. Fix those, and I can LGTM. 👍

ace-n · 2017-12-18T19:03:53Z

iot/http_example/cloudiot_http_example.js

@@ -166,6 +166,32 @@ function publishAsync (messageCount, numMessages) {
 }
 // [END iot_http_publish]

+// [START iot_http_getconfig]
+function getConfig (version) {
+  console.log('Getting config:');


Nit: combine these into one console.log call? e.g:

console.log(`Getting config from URL: ${urlBase}`);

ace-n · 2017-12-18T19:05:59Z

iot/http_example/cloudiot_http_example.js

+  const options = {
+    url: urlBase + '/config?local_version=' + version,
+    headers: {
+      'authorization': 'Bearer ' + authToken,


Nit: use template strings instead of concatenation? e.g:

`Bearer ${authToken}`

If you decide to make this change, please update the rest of your PR as well.

ace-n · 2017-12-18T19:08:51Z

iot/http_example/cloudiot_http_example.js

@@ -174,10 +200,14 @@ const devicePath = `projects/${argv.project_id}/locations/${argv.cloud_region}/r
 // The request path, set accordingly depending on the message type.
 const pathSuffix = argv.message_type === 'events'
    ? ':publishEvent' : ':setState';
-const url = `https://${argv.http_bridge_address}/v1beta1/${devicePath}${pathSuffix}`;
+const urlBase = `https://${argv.http_bridge_address}/v1beta1/${devicePath}`;


Move this to the top of the file, or use a let and take advantage of hoisting? Seeing this declared after the functions that use it may confuse people who are new to JS (and don't understand that global statements get executed first).

It uses the args, so at a minimum it needs to be after the argument parser... I have moved the variables up.

ace-n · 2017-12-18T19:10:42Z

iot/http_example/resources/README.md

+# Test public certificate files
+
+The public certificates in this folder are only provided for testing and should
+not be used for registering your devices.


Do we use these in our CI tests - if so, I assume they are sufficiently secure?

Yes, these certificates are used in our CI tests. The authorization credentials are revoked when the tests tear down.

ace-n · 2017-12-18T19:11:54Z

iot/http_example/system-test/cloudiot_http_example_test.js

+  output = await tools.runAsync(
+      `${cmd} --message_type=events --num_messages=1 --private_key_file=resources/rsa_private.pem --algorithm=RS256`, cwd);
+
+  t.regex(output, new RegExp(`Getting config`));


Nit: can this be t.regex(output, /Getting config/)?

ace-n · 2017-12-18T19:16:50Z

iot/http_example/system-test/cloudiot_http_example_test.js

+  const localDevice = `test-rsa-device`;
+  const localRegName = `${registryName}-rsa256`;
+  let output = await tools.runAsync(`${helper} setupIotTopic ${topicName}`, cwd);
+  output = await tools.runAsync(


Is the output = part required if you're not planning to use the output?

If not, please fix this throughout your PR.

…rm/nodejs-docs-samples into iot-http-tests

ace-n

LGTM, other than some additional nits. (Fix them, then merge away.)

ace-n · 2017-12-18T22:25:26Z

iot/http_example/system-test/cloudiot_http_example_test.js


-  output = await tools.runAsync(
+  let output = await tools.runAsync(


Nit: can these be const instead of let? (In general, use const for variables that are never reassigned to. If you make this change, please do so throughout your PR.)

ace-n · 2017-12-18T22:57:32Z

iot/http_example/cloudiot_http_example.js

+// must be in the format below.
+
+let iatTime = parseInt(Date.now() / 1000);
+let authToken = createJwt(argv.project_id, argv.private_key_file, argv.algorithm);


Nit: s/snake_case/camelCase/g

Co-authored-by: Benjamin E. Coe <[email protected]>

🤖 I have created a release \*beep\* \*boop\* --- ### [3.15.4](https://www.github.com/googleapis/nodejs-asset/compare/v3.15.3...v3.15.4) (2021-08-11) ### Bug Fixes * **build:** migrate to using main branch ([#534](https://www.github.com/googleapis/nodejs-asset/issues/534)) ([30fa2c4](https://www.github.com/googleapis/nodejs-asset/commit/30fa2c48abe3de2a9482ea5bde9a4f315a726b89)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: Benjamin E. Coe <[email protected]>

🤖 I have created a release \*beep\* \*boop\* --- ### [3.15.4](https://www.github.com/googleapis/nodejs-asset/compare/v3.15.3...v3.15.4) (2021-08-11) ### Bug Fixes * **build:** migrate to using main branch ([#534](https://www.github.com/googleapis/nodejs-asset/issues/534)) ([30fa2c4](https://www.github.com/googleapis/nodejs-asset/commit/30fa2c48abe3de2a9482ea5bde9a4f315a726b89)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: Benjamin E. Coe <[email protected]>

🤖 I have created a release \*beep\* \*boop\* --- ### [3.2.6](https://www.github.com/googleapis/nodejs-text-to-speech/compare/v3.2.5...v3.2.6) (2021-07-21) ### Bug Fixes * Updating WORKSPACE files to use the newest version of the Typescript generator. ([#534](https://www.github.com/googleapis/nodejs-text-to-speech/issues/534)) ([dee8dc2](https://www.github.com/googleapis/nodejs-text-to-speech/commit/dee8dc23024f67d5ad3888e2ea4b6829cbd4cae6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

gguuss added 2 commits December 11, 2017 16:46

Adds tests for HTTP device sample

821d10a

Merge branch 'master' into iot-http-tests

c143e9a

gguuss requested a review from ace-n December 13, 2017 20:30

ace-n reviewed Dec 18, 2017

View reviewed changes

gguuss added 2 commits December 18, 2017 12:47

Address review comments

b8f3a1f

Merge branch 'iot-http-tests' of https://github.com/GoogleCloudPlatfo…

95e6262

…rm/nodejs-docs-samples into iot-http-tests

ace-n approved these changes Dec 18, 2017

View reviewed changes

gguuss added 3 commits December 19, 2017 13:07

Address review comments

bfd5b29

Merge branch 'master' into iot-http-tests

b067b65

Merge branch 'master' into iot-http-tests

d118f5d

gguuss merged commit 5f54118 into master Dec 19, 2017

gguuss deleted the iot-http-tests branch December 19, 2017 23:39

snyk-bot mentioned this pull request Jan 31, 2020

[Snyk] Fix for 1 vulnerabilities leejh3224/nodejs-docs-samples#158

Open

snyk-bot mentioned this pull request Sep 5, 2020

[Snyk] Security upgrade twilio from 2.11.1 to 3.41.0 ninadhatkar/nodejs-docs-samples#35

Open

snyk-bot mentioned this pull request Dec 23, 2020

[Snyk] Security upgrade @google-cloud/spanner from 2.2.1 to 3.0.0 leejh3224/nodejs-docs-samples#289

Open

snyk-bot mentioned this pull request Sep 6, 2021

[Snyk] Security upgrade @google-cloud/spanner from 2.2.1 to 3.0.0 leejh3224/nodejs-docs-samples#365

Open

snyk-bot mentioned this pull request Jan 12, 2022

[Snyk] Security upgrade @google-cloud/spanner from 2.2.1 to 3.0.0 leejh3224/nodejs-docs-samples#429

Open

snyk-bot mentioned this pull request Feb 10, 2022

[Snyk] Security upgrade @google-cloud/spanner from 2.2.1 to 3.0.0 leejh3224/nodejs-docs-samples#462

Open

snyk-bot mentioned this pull request Apr 7, 2022

[Snyk] Security upgrade twilio from 2.11.1 to 3.41.0 ninadhatkar/nodejs-docs-samples#188

Open

ace-n pushed a commit that referenced this pull request Nov 11, 2022

docs: fix typo in sample (#534)

5ee4f18

Co-authored-by: Benjamin E. Coe <[email protected]>

ace-n mentioned this pull request Nov 11, 2022

migrate code from googleapis/nodejs-automl #2841

Merged

ace-n pushed a commit that referenced this pull request Nov 11, 2022

docs: fix typo in sample (#534)

7478dfe

Co-authored-by: Benjamin E. Coe <[email protected]>

ace-n pushed a commit that referenced this pull request Nov 14, 2022

docs: fix typo in sample (#534)

5ff5e44

Co-authored-by: Benjamin E. Coe <[email protected]>

ace-n pushed a commit that referenced this pull request Nov 15, 2022

docs: fix typo in sample (#534)

b87b28a

Co-authored-by: Benjamin E. Coe <[email protected]>

ace-n pushed a commit that referenced this pull request Nov 15, 2022

docs: fix typo in sample (#534)

5b6387a

Co-authored-by: Benjamin E. Coe <[email protected]>

ahrarmonsur pushed a commit that referenced this pull request Nov 16, 2022

fix(build): migrate to using main branch (#534)

465853a

ace-n pushed a commit that referenced this pull request Nov 17, 2022

docs: fix typo in sample (#534)

5658559

Co-authored-by: Benjamin E. Coe <[email protected]>

ace-n pushed a commit that referenced this pull request Nov 17, 2022

docs: fix typo in sample (#534)

a563d05

Co-authored-by: Benjamin E. Coe <[email protected]>

ace-n mentioned this pull request Nov 17, 2022

migrate code from googleapis/nodejs-translate #2862

Merged

ahrarmonsur pushed a commit that referenced this pull request Nov 17, 2022

docs: fix typo in sample (#534)

0e6aa43

Co-authored-by: Benjamin E. Coe <[email protected]>

ninadhatkar mentioned this pull request Nov 28, 2023

[Snyk] Security upgrade twilio from 2.11.1 to 3.41.0 ninadhatkar/nodejs-docs-samples#351

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds tests for HTTP device sample #534

Adds tests for HTTP device sample #534

gguuss commented Dec 12, 2017

codecov bot commented Dec 13, 2017 •

edited

Loading

ace-n left a comment

ace-n Dec 18, 2017

ace-n Dec 18, 2017

ace-n Dec 18, 2017

gguuss Dec 18, 2017

ace-n Dec 18, 2017

gguuss Dec 18, 2017

ace-n Dec 18, 2017

gguuss Dec 18, 2017

ace-n Dec 18, 2017

gguuss Dec 18, 2017

ace-n left a comment

ace-n Dec 18, 2017

ace-n Dec 18, 2017


		output = await tools.runAsync(
		let output = await tools.runAsync(

Adds tests for HTTP device sample #534

Adds tests for HTTP device sample #534

Conversation

gguuss commented Dec 12, 2017

codecov bot commented Dec 13, 2017 • edited Loading

Codecov Report

ace-n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ace-n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 13, 2017 •

edited

Loading