Add new "quickstart" samples#547
Conversation
|
It's sad to see that there are no tests for these, especially considering these will be the first samples users try out. |
I'm willing to write tests, but I need a crash course on testing python-docs-samples. |
I can add them, they're just inherently hard to test. As soon as you import these modules they do stuff (side-effects), this is why all of samples are in functions - so that tests can setup fixtures to mock whatever is needed or setup environment variables before calling the function. You've also hard-coded the project id, which means I have to do at least some mocking. It's also inconsistent with all of the parameters shown in other samples (which are just function args). |
|
I recognize that they're harder to test (the tests for Node.js versions of these samples were a pain). Perhaps @omaray can concede some simplicity for more maintainable/testable samples? i.e. wrap in a function? Hard-coding the project ID is a "product requirement" for these samples (these "quickstart" style samples will be the only ones that do so, all the rest can infer from the environment). Also, I tried TESTING.md, but |
|
@jonparrott happy to chat in person about this. |
|
We were hoping this week if possible. But we can also check-in something for now that works (that we test manually to confirm) and then add the test next week. That wouldn't be a concern to me. By the way, @jmdobry is using the "gcloud" module in his examples. Can we now switch to using the google-cloud-{api} one instead? So |
With python, can the test do the setup stuff before import the sample? (That's essentially what we do in Node.js.) Are imports required to be the first statements in a file? |
We can do that, it's just pretty hacky.
That would be fine. |
|
@jmdobry if you rebase on master, you can use google-cloud (and tests should pass) |
8bc413d to
fef2506
Compare
|
Done |
fef2506 to
470e41b
Compare
|
I removed the explicit project IDs. |
|
I wrapped each snippet in a method definition, does that make it easier for you to test? I also added snippets for Language and Vision. Speech is the only one missing ( |
|
Also, I ran all of the samples manually to make sure they work. Except for Speech, this PR now has "quickstart" sample parity with GoogleCloudPlatform/ruby-docs-samples#77 and the corresponding Node.js samples (which are already on |
These are intended to be simple examples that explicitly show:
Products covered:
For reference, see the corresponding samples in other languages:
masterin variousquickstart.jsfiles)Thanks!