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

build: fix system tests, move to Kokoro #372

Merged
merged 26 commits into from
Oct 25, 2019
Merged

build: fix system tests, move to Kokoro #372

merged 26 commits into from
Oct 25, 2019

Conversation

busunkim96
Copy link
Contributor

@busunkim96 busunkim96 commented Sep 19, 2019

  • Fix system tests
  • Use nox for everything (remove tox.ini).
  • Add Kokoro configs.
  • Add black.
  • Use same lint standards as googleapis/google-cloud-python

You can ignore changes to source code and test files in google/ and tests/, they are the result of running black.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 19, 2019
@busunkim96 busunkim96 force-pushed the use-new-nox branch 4 times, most recently from 058fc1d to 84fc5f1 Compare October 8, 2019 23:53
@busunkim96 busunkim96 marked this pull request as ready for review October 9, 2019 00:05
@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 9, 2019
Comment on lines +49 to +50
python3.6 -m nox
python3.6 -m nox -f system_tests/noxfile.py
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The system test noxfile seemed complicated enough that I didn't want to pull it into the top-level noxfile. I also experimented with having nox run nox, but there was some weirdness there.

@busunkim96 busunkim96 requested a review from a team October 10, 2019 23:58
.flake8 Outdated Show resolved Hide resolved
.kokoro/publish-docs.sh Show resolved Hide resolved
.repo-metadata.json Outdated Show resolved Hide resolved
google/auth/_cloud_sdk.py Outdated Show resolved Hide resolved
@busunkim96 busunkim96 mentioned this pull request Oct 21, 2019
"requests",
"requests-oauthlib",
"urllib3",
"cryptography",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these seem required for the actual library to function. Those should be defined in setup.py, not here, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a straight copy from the tox.ini. Some of them might not be needed anymore though. I'll go through and check

@@ -1,65 +0,0 @@
# Copyright 2016 Google Inc.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intended to delete this file? How are we system testing user-based credentials files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of scripts/obtain_user_auth.py was intentional. The current script uses oauth2client to make the credentials. I added instructions on how to get the credentials using gcloud in the 'Running system tests' section of the CONTRIBUTING.rst.

https://github.com/googleapis/google-auth-library-python/blob/use-new-nox/CONTRIBUTING.rst#running-system-tests

system_tests/noxfile.py Outdated Show resolved Hide resolved
@busunkim96 busunkim96 merged commit 65e33c0 into master Oct 25, 2019
@busunkim96 busunkim96 deleted the use-new-nox branch October 25, 2019 17:45
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.

4 participants