Skipping system tests when credentials env. var is unset. by dhermes · Pull Request #3475 · googleapis/google-cloud-python

dhermes · 2017-06-05T23:24:09Z

This is labeled "don't merge" because it is wrong in it's current state.

So @lukesneeringer @tseaver @jonparrott WDYT the "correct" form should be? We don't want it to raise on PRs, we want it to gracefully do nothing. Otherwise, this is a helpful message and status for people running tests locally with a "bad" setup.

Sign in to view

 if not os.environ.get('GOOGLE_APPLICATION_CREDENTIALS', ''):
- return
+ raise nox.command.CommandFailed(
+ reason='Credentials must be set via environment variable.')


lukesneeringer · 2017-06-13T15:11:15Z

Thoughts on this (sorry for the delay):

It is totally possible to read one of the CircleCI / AppVeyor pull request variables and using those as the basis to skip. (I am less convinced it is a good idea.)
I am not as convinced as @dhermes that you always want to run system tests in local testing. In particular, I would expect external contributors generally do not want to do so, or want to run them for only the package they are working on.
We could add some kind of skipped state to nox. It would be far more valuable for us to report "5 sessions passed, 2 skipped" than "7 sessions passed". I think that would be my preference. It also follows @jonparrott's desire to "generally follow py.test as much as possible" as this is definitely a well-understood testing feature. It is also a useful feature lacking in tox. :-)

theacodes · 2017-06-13T15:22:00Z

I'm totally happy to accept a PR to nox to add `session.skip(reason)`.

lukesneeringer · 2017-06-13T17:23:12Z

@dhermes Does that work for you?

dhermes · 2017-06-13T17:31:01Z

Yes I am OK with that. I don't have bandwidth to add that feature to nox though

theacodes · 2017-06-13T17:39:47Z

Y'all can file a bug and I'll try to get to it while I'm bored at the hotel this week.

dhermes · 2017-06-13T17:41:57Z

Donezors: wntrblm/nox#34

dhermes · 2017-06-28T17:24:45Z

I just removed the "do not merge" label.

@jonparrott I think this is read to go. PTAL?

@lukesneeringer ISTM that we'll need our CircleCI image to have the latest version of nox-automation, what's the best way to go about that?

@duggelz I have intentionally avoided linked to CONTRIBUTING.rst for the time being (though I think it's a great idea). This is because the doc is months out-of-date.

dhermes · 2017-06-28T19:37:06Z

Update: currently waiting on @lukesneeringer to rebuild our Docker image, so we can ditch the pip install --upgrade nox-automation that I added.

(It's not a big hurry to merge this.)

dhermes · 2017-06-29T17:20:58Z

OK @lukesneeringer got us googleapis/nox:0.17.0 so this thing is ready to go once green.

…#3475)

dhermes added build do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Jun 5, 2017

dhermes assigned lukesneeringer, tseaver and theacodes Jun 5, 2017

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 5, 2017

duggelz reviewed Jun 6, 2017
View reviewed changes

Comment threadbigquery/nox.py Outdated
if not os.environ.get('GOOGLE_APPLICATION_CREDENTIALS', ''):
return
raise nox.command.CommandFailed(
reason='Credentials must be set via environment variable.')
This comment was marked as spam.
Sign in to view
This comment was marked as spam.
Sign in to view

dhermes mentioned this pull request Jun 13, 2017
Add session.skip(reason)wntrblm/nox#34
Closed

dhermes force-pushed the fail-tests-without-creds branch from 6410981 to ab0bc35Compare June 28, 2017 17:23

dhermes removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jun 28, 2017

theacodes approved these changes Jun 28, 2017
View reviewed changes

tseaver approved these changes Jun 28, 2017
View reviewed changes

dhermes changed the title ~~Failing system tests when credentials env. var is unset.~~Skipping system tests when credentials env. var is unset.Jun 28, 2017

Skipping system tests when credentials env. var is unset.
2685c36

dhermes force-pushed the fail-tests-without-creds branch from 6519b5c to 2685c36Compare June 29, 2017 17:19

dhermes mentioned this pull request Jun 29, 2017
Remove apt-get install openssl from CircleCI config. #3567
Merged

dhermes merged commit b9cb6d1 into googleapis:masterJun 29, 2017

dhermes deleted the fail-tests-without-creds branch June 29, 2017 17:56

tseaver pushed a commit that referenced this pull request Jul 10, 2017
Skipping system tests when credentials env. var is unset. (#3475)
45b32b6

dhermes mentioned this pull request Jul 14, 2017
Speech GAPIC to master #3607
Merged

lukesneeringer mentioned this pull request Aug 11, 2017
system_test nox sessions shouldn't ALWAYS "pass" with no config #3248
Closed

landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 21, 2017
Skipping system tests when credentials env. var is unset. (googleapis…
a6154a3
…#3475)

landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
Skipping system tests when credentials env. var is unset. (googleapis…
9c2b37b
…#3475)

landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017
Skipping system tests when credentials env. var is unset. (googleapis…
262f55a
…#3475)

atulep pushed a commit that referenced this pull request Apr 3, 2023
Skipping system tests when credentials env. var is unset. (#3475)
bead634

atulep pushed a commit that referenced this pull request Apr 18, 2023
Skipping system tests when credentials env. var is unset. (#3475)
ccc74bb

parthea pushed a commit that referenced this pull request Jul 6, 2023
Skipping system tests when credentials env. var is unset. (#3475)
eaabe16

parthea pushed a commit that referenced this pull request Oct 21, 2023
Skipping system tests when credentials env. var is unset. (#3475)
4f0a678

parthea pushed a commit that referenced this pull request Oct 21, 2023
Skipping system tests when credentials env. var is unset. (#3475)
0916c9a

parthea pushed a commit that referenced this pull request Oct 21, 2023
Skipping system tests when credentials env. var is unset. (#3475)
fcfdf18

parthea pushed a commit that referenced this pull request Oct 22, 2023
Skipping system tests when credentials env. var is unset. (#3475)
b6aa9db

parthea pushed a commit that referenced this pull request Nov 22, 2025
Skipping system tests when credentials env. var is unset. (#3475)
cec0f09

parthea pushed a commit that referenced this pull request Nov 24, 2025
Skipping system tests when credentials env. var is unset. (#3475)
3beb2f9

parthea pushed a commit that referenced this pull request Nov 24, 2025
Skipping system tests when credentials env. var is unset. (#3475)
b4c3c33

parthea pushed a commit that referenced this pull request Nov 24, 2025
Skipping system tests when credentials env. var is unset. (#3475)
acd3998

parthea pushed a commit that referenced this pull request Mar 2, 2026
Skipping system tests when credentials env. var is unset. (#3475)
06333cb

parthea pushed a commit that referenced this pull request Mar 9, 2026
Skipping system tests when credentials env. var is unset. (#3475)
b92d07f

parthea pushed a commit that referenced this pull request Mar 9, 2026
Skipping system tests when credentials env. var is unset. (#3475)
eea4326

Navigation Menu

Uh oh!

Skipping system tests when credentials env. var is unset. - #3475

Skipping system tests when credentials env. var is unset.#3475
dhermes merged 1 commit into
googleapis:masterfrom
dhermes:fail-tests-without-creds

dhermes commented Jun 5, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

lukesneeringer commented Jun 13, 2017•
edited
Loading

Uh oh!

theacodes commented Jun 13, 2017 via email •
edited by lukesneeringer
Loading

Uh oh!

lukesneeringer commented Jun 13, 2017

Uh oh!

dhermes commented Jun 13, 2017

Uh oh!

theacodes commented Jun 13, 2017 via email •
edited by dhermes
Loading

Uh oh!

dhermes commented Jun 13, 2017

Uh oh!

dhermes commented Jun 28, 2017

Uh oh!

dhermes commented Jun 28, 2017

Uh oh!

dhermes commented Jun 29, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

dhermes commented Jun 5, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

lukesneeringer commented Jun 13, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theacodes commented Jun 13, 2017 via email • edited by lukesneeringer Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukesneeringer commented Jun 13, 2017

Uh oh!

dhermes commented Jun 13, 2017

Uh oh!

theacodes commented Jun 13, 2017 via email • edited by dhermes Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhermes commented Jun 13, 2017

Uh oh!

dhermes commented Jun 28, 2017

Uh oh!

dhermes commented Jun 28, 2017

Uh oh!

dhermes commented Jun 29, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lukesneeringer commented Jun 13, 2017•
edited
Loading

theacodes commented Jun 13, 2017 via email •
edited by lukesneeringer
Loading

theacodes commented Jun 13, 2017 via email •
edited by dhermes
Loading