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.

Sign up for GitHub

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

Jump to bottom

tests: revise test to run without setting credentials in env #579

Merged
merged 3 commits into from Sep 7, 2021
Merged

tests: revise test to run without setting credentials in env #579

merged 3 commits into from Sep 7, 2021

Conversation

cojenco
Copy link
Contributor

@cojenco cojenco commented Sep 2, 2021

Part of #570 and #565

@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/python-storage API. label Sep 2, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 2, 2021
@cojenco cojenco marked this pull request as ready for review September 2, 2021 21:25
@cojenco cojenco requested a review from a team as a code owner September 2, 2021 21:25
tseaver
tseaver reviewed Sep 3, 2021
View reviewed changes
Copy link
Contributor

@tseaver tseaver left a comment

Choose a reason for hiding this comment

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

@cojenco the need to use a "real" Client here has a bit of "code smell": The Connection class shouldn't need to use Client for anything, even tests. Would a better fix for #565 be to have the Batch class construct a copy of the original ClientInfo?

@cojenco
Copy link
Contributor Author

cojenco commented Sep 3, 2021

@tseaver Makes sense, thanks! I've revised the test to mock out Client. ClientInfo is propagated from the Client passed into the Batch class constuctor.

@cojenco cojenco merged commit 98a0e62 into googleapis:main Sep 7, 2021
@cojenco cojenco deleted the test-revision-565 branch September 10, 2021 21:32
cojenco added a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
@cojenco
tests: revise test to run without setting credentials in env (googlea…
5cc43f6 
…pis#579)

* tests: revise test to run without setting credentials in env

* address review comments and mock client
cojenco added a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
@cojenco
tests: revise test to run without setting credentials in env (googlea…
02d1bdf 
…pis#579)

* tests: revise test to run without setting credentials in env

* address review comments and mock client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tseaver tseaver tseaver left review comments

@busunkim96 busunkim96 busunkim96 approved these changes

Assignees
No one assigned
Labels
api: storage Issues related to the googleapis/python-storage API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@cojenco @tseaver @busunkim96