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

Is there any way to make our example code in docstrings part of the test suite? #598

Closed
jgeewax opened this issue May 14, 2015 · 12 comments · Fixed by #627
Closed

Is there any way to make our example code in docstrings part of the test suite? #598

jgeewax opened this issue May 14, 2015 · 12 comments · Fixed by #627
Assignees
Labels
type: question Request for information or clarification. Not an issue.
Milestone

Comments

@jgeewax
Copy link
Contributor

jgeewax commented May 14, 2015

I'm worried about code rot and typos -- is there any way we can make it so that the snippets we have are actually tested ? Could we pull them out of a unit test somewhere? or a test suite that is specifically samples and we load based on an ID of sorts?

@jgeewax jgeewax added type: question Request for information or clarification. Not an issue. docs labels May 14, 2015
@jgeewax jgeewax added this to the Core Future milestone May 14, 2015
@ryanseys
Copy link
Contributor

We don't have assertions in our code examples and we probably shouldn't. This would make our examples gross. If they want examples that definitely work and have assertions, they can look at the tests. :)

@jgeewax
Copy link
Contributor Author

jgeewax commented May 14, 2015

We don't have assertions in our code examples and we probably shouldn't.

Maybe we could put asserts in there and strip them out as part of populating the examples?

If they want examples that definitely work and have assertions, they can look at the tests. :)

That seems kind of crappy... I think we owe users of the library examples that work... :(

@brendandburns
Copy link

In kubernetes open source, we explicitly have a unit test that scrapes all
of our docs looking for examples like this and making sure that they still
work.

It's 100% do-able, and I think we should prioritize it. The number of out
of date examples, when I was initially learning the Google Cloud Platform
was really bad.

Brendan
On May 14, 2015 2:12 PM, "JJ Geewax" notifications@github.com wrote:

We don't have assertions in our code examples and we probably shouldn't.

Maybe we could put asserts in there and strip them out as part of
populating the examples?

If they want examples that definitely work and have assertions, they can
look at the tests. :)

That seems kind of crappy... I think we owe users of the library examples
that work... :(


Reply to this email directly or view it on GitHub
#598 (comment)
.

@ryanseys
Copy link
Contributor

Personally, I don't like the idea of docs and tests being so closely linked. We talk to computers with tests, we talk to humans with docs. I like the flexibility that our docs give us and the ability to be clear and concise. If we change this, our examples need to be as verbose as our tests in which case, why don't we just scrap the examples and copy and paste our tests into the docs?

I do understand the appeal, don't get me wrong, so if we can make this not as painful as I envision it, then I'd be happy to explore the idea.

@brendandburns can you link me to code on how this is done in kubernetes?

As an aside, I think our developers.google.com docs are always out of date. :( The struggle is so real.

@jgeewax
Copy link
Contributor Author

jgeewax commented May 14, 2015

If we change this, our examples need to be as verbose as our tests

To be more clear: I'm not saying our docs need to be as verbose as tests. I'm saying that they need to work -- always.

I'd really love it if we had a check at pull-request time to make sure all the examples still work... How we do this is in the air, but the goal is that if I were to check in a typo, Travis would reject it...

As an aside, I think our developers.google.com docs are always out of date. :( The struggle is so real.

And it sucks. Big time. This is just adding fuel to the fire of "let's make sure our examples work..."

@brendandburns
Copy link

Here is the code:

https://github.com/GoogleCloudPlatform/kubernetes/blob/master/examples/examples_test.go

The basic approach is:

  • Scrape through the docs, look for things that start with ```
  • Try and extract that thingy
  • Try to parse that data object into a sensible object
  • Validate with a rule in the test that indicates what the object
    should be.

Concretely for developers.google.com, I think this would look like, writing
a unit test for each API, that scrapes through the relevant
developer.google.com content, extracts source code, and attempts to compile
it using the appropriate library. That way, if a developer modifies
something that breaks the example, they have to fix the docs as well as
the change their making.

A different option would be to figure out a way to automatically include
the source code from original source files, if you do that, then you can
just write traditional unit tests that live alongside the example source
files.

On Thu, May 14, 2015 at 2:39 PM, JJ Geewax notifications@github.com wrote:

If we change this, our examples need to be as verbose as our tests

To be more clear: I'm not saying our docs need to be as verbose as tests.
I'm saying that they need to work -- always.

I'd really love it if we had a check at pull-request time to make sure all
the examples still work... How we do this is in the air, but the goal is
that if I were to check in a typo, Travis would reject it...

As an aside, I think our developers.google.com docs are always out of
date. :( The struggle is so real.

And it sucks. Big time. This is just adding fuel to the fire of "let's
make sure our examples work..."


Reply to this email directly or view it on GitHub
#598 (comment)
.

@tmatsuo
Copy link
Contributor

tmatsuo commented May 14, 2015

@elibixby

FYI, for cloud.google.com (cloud instance of developers.google.com) sample code, we will go to the latter option.

@ryanseys
Copy link
Contributor

Like thinking hypothetically, we should just split each line and run it through the node REPL. If it doesn't throw up somewhere along the way, good2go?

@jgeewax
Copy link
Contributor Author

jgeewax commented May 14, 2015

You mean all the lines as a whole right? Ie, this should fail:

var gcloud = require('gcloud');
var dataset = gcloud.datastore.dataset();
dataset.save({
  key: datastore.key('Company'),
  data: {name: 'Google'}
});

@ryanseys
Copy link
Contributor

Oh yeah sorry, it's too early. That sounds reasonable yes?

@jgeewax
Copy link
Contributor Author

jgeewax commented May 14, 2015

If the snippet runs successfully, that's a good thing...

I think it's safe to assume that the methods we're calling work as expected, and are tested to do so in a unit test. So the following "test" wouldn't be in the docs:

var key = dataset.key('Company');
assert(key.path[0] == 'Company');

We're not trying to check that methods do the right things (ie, that creating a key with 'Company' as the first argument sets the kind). We're trying to check that our examples use methods in the right ways. That is, I'm looking primary for typos, invalid method parameters (ie, dataset.key('too', 'many', 'arguments')), invalid method names (ie, datastore.key('Company') instead of dataset.key('Company')), and other things like that.

Figuring out if calling the right method does what it should do is a job for tests IMO. And we always have the "report an issue" button and pull requests for people who find broken code that slips into the gaps...

@brendandburns , can you comment and tell me if I'm way out of line here?

@brendandburns
Copy link

Yeah, I agree. You're looking for code that doesn't compile or is out of
date relative to the current state of APIs.

Functional testing to make sure it works is definitely a nice to have, not
a requirement.
On May 14, 2015 3:36 PM, "JJ Geewax" notifications@github.com wrote:

If the snippet runs successfully, that's a good thing...

I think it's safe to assume that the methods we're calling work as
expected, and are tested to do so in a unit test. So the following "test"
wouldn't be in the docs:

var key = dataset.key('Company');
assert(key.path[0] == 'Company');

We're not trying to check that methods do the right things (ie, that
creating a key with 'Company' as the first argument sets the kind). We're
trying to check that our examples use methods in the right ways. That is,
I'm looking primary for typos, invalid method parameters (ie, dataset.key('too',
'many', 'arguments')), invalid method names (ie, datastore.key('Company')
instead of dataset.key('Company')), and other things like that.

Figuring out if calling the right method does what it should do is a job
for tests IMO. And we always have the "report an issue" button and pull
requests for people who find broken code that slips into the gaps...

@brendandburns https://github.com/brendandburns , can you comment and
tell me if I'm way out of line here?


Reply to this email directly or view it on GitHub
#598 (comment)
.

sofisl pushed a commit that referenced this issue Oct 11, 2022
sofisl pushed a commit that referenced this issue Oct 13, 2022
sofisl pushed a commit that referenced this issue Nov 10, 2022
#598)

* build(node): add feat in node post-processor to add client library version number in snippet metadata

Co-authored-by: Benjamin E. Coe <bencoe@google.com>
Source-Link: googleapis/synthtool@d337b88
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:d106724ad2a96daa1b8d88de101ba50bdb30b8df62ffa0aa2b451d93b4556641
sofisl pushed a commit that referenced this issue Nov 10, 2022
* chore: update codeowners

https://github.com/orgs/googleapis/teams/aap-dpes

* 🦉 Updates from OwlBot

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>
Co-authored-by: Benjamin E. Coe <bencoe@google.com>
sofisl pushed a commit that referenced this issue Nov 17, 2022
Group for engineers contributing to compute library.

CC: @vchudnov-g
sofisl pushed a commit that referenced this issue Sep 13, 2023
* chore: update v2.14.2 gapic-generator-typescript

Committer: @summer-ji-eng
PiperOrigin-RevId: 434859890

Source-Link: googleapis/googleapis@bc2432d

Source-Link: googleapis/googleapis-gen@930b673
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiOTMwYjY3MzEwM2U5MjUyM2Y4Y2ZlZDM4ZGVjZDdkM2FmYWU4ZWJlNyJ9

* 🦉 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants