Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify @example docs on npm test #627

Merged
merged 2 commits into from
May 29, 2015
Merged

Verify @example docs on npm test #627

merged 2 commits into from
May 29, 2015

Conversation

ryanseys
Copy link
Contributor

Fixes #598
Fixes #622
Fixes #623

I really wanted to build this because it seemed like an interesting problem and this builds on our already-epic docs to make them more epic.

This adds an additional test which will run with npm test like normal, though I would suggest we consider changing npm test to also generate docs, then run the tests to ensure the test remains up to date with the docs generated.

The idea is that we read in docs/**/*.json files and iterate through every appropriate @example tag, extract the code string, and run it inside a node.js vm sandbox, ensuring that it doesn't throw an error along the way. This means that it will check for general syntax errors, as well as calling methods with the wrong number of parameters or missing parameters or just plain incorrect parameters.

A lot of this PR is cleaning up the docs so the test passes! Yeah, it caught a wide range of issues! 👍

One thing to note is that it has no way of running anything asynchronously or waiting in anyway for any async function to be called in the VM because we'd have to then inject our own code into the existing examples and that would be quite difficult to guess.

For example Dataset#runInTransaction (which you'll see a lot of in this PR) actually is an async function and so it makes it impossible to test anything that is included in the callback of that function, and unfortunately the only way we recommend they create a transaction object is by running dataset.runInTransaction() so I'm not sure what we should do to mitigate this issue and test our transaction object. I'll comment on the PR where this issue takes place.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 25, 2015
* numDonutsServed: 45, // detect number type (int or double)
* founded: new Date('Tue May 12 2015 15:30:00 GMT-0400 (EDT)'), // dates
* // founded: new Date('Tue May 12 2015 15:30:00 GMT-0400 (EDT)'), // dates

This comment was marked as spam.

@jgeewax
Copy link
Contributor

jgeewax commented May 26, 2015

This is awesome. I totally get it that we can't inspect inside callbacks... maybe there's some magic we can figure out later, but for now this is a huge step forward. 👍

@stephenplusplus
Copy link
Contributor

This is amazing. This is the first time I've seen anything like this in OSS/ever. @jgeewax is there some kind of Google Dev Blog we can write about this on after it's merged?

* // Commit the transaction.
* transaction.finalize(function(err, apiResponse) {});
* dataset.runInTransaction(function(transaction, done) {
* transaction.begin(function(err) {

This comment was marked as spam.

This comment was marked as spam.

@ryanseys
Copy link
Contributor Author

Adding don't merge so I can continue working on this today to investigate mitigating some of the issues I've made apparent here with the current version.

@ryanseys
Copy link
Contributor Author

Whoa! This docs test found a real live bug! Our transaction#rollback didn't attach the transaction id like it should have. I added a unit test to catch it :)

@@ -143,10 +141,10 @@ Transaction.prototype.rollback = function(callback) {
* Commit the remote transaction and finalize the current transaction instance.
*
* @param {function} callback - The callback function.
* @private

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor

Whoa! This docs test found a real live bug! Our transaction#rollback didn't attach the transaction id like it should have. I added a unit test to catch it :)

Nice!

@ryanseys
Copy link
Contributor Author

PTAL. Should test all our examples, including ignored and private ones.
Also privatized Transaction#begin_ and Transaction#commit_.

});

it('should run docs examples without errors', function() {
for (var f in FILES) {

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor

This isn't a problem in this PR, but I just discovered something I must have messed up before:

https://github.com/ryanseys/gcloud-node/blob/72dc556b8911972fe6afdd1aec9fe4e774d4cc27/lib/datastore/transaction.js#L266

Those examples shouldn't show delete taking a callback. Feel free to fix while you're in there, or I can always get to it after.

].join('\n'));
}

var sandbox = {

This comment was marked as spam.

This comment was marked as spam.

@@ -411,7 +411,7 @@ function valueToProperty(v) {
p.blob_value = v;
return p;
}
if (v instanceof Array) {
if (Array.isArray(v)) {

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor

This is awesome. You made history.

stephenplusplus added a commit that referenced this pull request May 29, 2015
@stephenplusplus stephenplusplus merged commit 5aff671 into googleapis:master May 29, 2015
@ryanseys ryanseys deleted the verify-docs branch May 29, 2015 19:17
@ryanseys
Copy link
Contributor Author

Yay! This thing is awesome! :) I'm excited to see it break our builds with no remorse.

chingor13 pushed a commit that referenced this pull request Aug 22, 2022
* feat: Add SavedQuery CURD support
feat: Add tags support
feat!:*Add RelatedAsset and deprecate RelatedAssets for relationship GA

*The previous representation of the relationship feature is deprecated and unimplemented. The RelatedAsset message represents the new stable format.

PiperOrigin-RevId: 449306805

Source-Link: googleapis/googleapis@3d7bd9d

Source-Link: googleapis/googleapis-gen@71a93d0
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNzFhOTNkMDVkNjA3NjI3MWQwNGI3NTkyZjdmYWQwZDNmMGM3YTA0MCJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
chingor13 pushed a commit that referenced this pull request Aug 22, 2022
* feat: Add SavedQuery CURD support
feat: Add tags support
feat!:*Add RelatedAsset and deprecate RelatedAssets for relationship GA

*The previous representation of the relationship feature is deprecated and unimplemented. The RelatedAsset message represents the new stable format.

PiperOrigin-RevId: 449306805

Source-Link: googleapis/googleapis@3d7bd9d

Source-Link: googleapis/googleapis-gen@71a93d0
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNzFhOTNkMDVkNjA3NjI3MWQwNGI3NTkyZjdmYWQwZDNmMGM3YTA0MCJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
chingor13 pushed a commit that referenced this pull request Aug 22, 2022
* feat: Add SavedQuery CURD support
feat: Add tags support
feat!:*Add RelatedAsset and deprecate RelatedAssets for relationship GA

*The previous representation of the relationship feature is deprecated and unimplemented. The RelatedAsset message represents the new stable format.

PiperOrigin-RevId: 449306805

Source-Link: googleapis/googleapis@3d7bd9d

Source-Link: googleapis/googleapis-gen@71a93d0
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNzFhOTNkMDVkNjA3NjI3MWQwNGI3NTkyZjdmYWQwZDNmMGM3YTA0MCJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
sofisl pushed a commit that referenced this pull request Nov 10, 2022
- [ ] Regenerate this pull request now.

PiperOrigin-RevId: 474338479

Source-Link: googleapis/googleapis@d5d35e0

Source-Link: googleapis/googleapis-gen@efcd3f9
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZWZjZDNmOTM5NjJhMTAzZjY4ZjAwM2UyYTFlZWNkZTZmYTIxNmEyNyJ9
@release-please release-please bot mentioned this pull request Nov 10, 2022
sofisl pushed a commit that referenced this pull request Nov 11, 2022
🤖 I have created a release *beep* *boop*
---


## [4.1.0](googleapis/nodejs-dataproc@v4.0.1...v4.1.0) (2022-09-22)


### Features

* Add support for Dataproc metric configuration ([#626](googleapis/nodejs-dataproc#626)) ([79a4958](googleapis/nodejs-dataproc@79a4958))
* Support regapic LRO ([350626e](googleapis/nodejs-dataproc@350626e))


### Bug Fixes

* Allow passing gax instance to client constructor ([#612](googleapis/nodejs-dataproc#612)) ([caba80a](googleapis/nodejs-dataproc@caba80a))
* **deps:** Do not depend on protobufjs ([#607](googleapis/nodejs-dataproc#607)) ([4b62ef6](googleapis/nodejs-dataproc@4b62ef6))
* **deps:** Roll back dependency @google-cloud/dataproc to ^4.0.0 ([#627](googleapis/nodejs-dataproc#627)) ([e1c6d8b](googleapis/nodejs-dataproc@e1c6d8b))
* Do not import the whole google-gax from proto JS ([#1553](https://github.com/googleapis/nodejs-dataproc/issues/1553)) ([#617](googleapis/nodejs-dataproc#617)) ([2e2bea1](googleapis/nodejs-dataproc@2e2bea1))
* Preserve default values in x-goog-request-params header ([#620](googleapis/nodejs-dataproc#620)) ([484a023](googleapis/nodejs-dataproc@484a023))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
sofisl pushed a commit that referenced this pull request Nov 11, 2022
fix: use google-gax v3.3.0
Source-Link: googleapis/synthtool@c73d112
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:b15a6f06cc06dcffa11e1bebdf1a74b6775a134aac24a0f86f51ddf728eb373e
sofisl pushed a commit that referenced this pull request Nov 11, 2022
🤖 I have created a release *beep* *boop*
---


## [3.0.2](googleapis/nodejs-monitoring@v3.0.1...v3.0.2) (2022-09-09)


### Bug Fixes

* Do not import the whole google-gax from proto JS ([#1553](https://github.com/googleapis/nodejs-monitoring/issues/1553)) ([#627](googleapis/nodejs-monitoring#627)) ([647e8ab](googleapis/nodejs-monitoring@647e8ab))
* use google-gax v3.3.0 ([647e8ab](googleapis/nodejs-monitoring@647e8ab))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
sofisl pushed a commit that referenced this pull request Nov 11, 2022
* feat: Add SavedQuery CURD support
feat: Add tags support
feat!:*Add RelatedAsset and deprecate RelatedAssets for relationship GA

*The previous representation of the relationship feature is deprecated and unimplemented. The RelatedAsset message represents the new stable format.

PiperOrigin-RevId: 449306805

Source-Link: googleapis/googleapis@3d7bd9d

Source-Link: googleapis/googleapis-gen@71a93d0
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNzFhOTNkMDVkNjA3NjI3MWQwNGI3NTkyZjdmYWQwZDNmMGM3YTA0MCJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
sofisl added a commit that referenced this pull request Jan 24, 2023
Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com>
sofisl added a commit that referenced this pull request Jan 25, 2023
Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transaction Documentation Is there any way to make our example code in docstrings part of the test suite?
4 participants