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

chore(bigquery): remove duplicate test dependencies #9503

Merged
merged 2 commits into from
Oct 24, 2019

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Oct 20, 2019

Fixes #9502.

This PR removes redundancy in BigQuery's noxfile that results in test_utils being installed twice.

(modifying the noxfile directly, as it is not generated by the synth tool in BigQuery)

How to test

  • Run the following nox sessions: snippets and system. You do not have to wait for them to complete, just make sure the setup phase is completed and the relevant tests start running.

Actual result (before the fix):
In both cases test_utils are installed twice. In the output:

...
nox > pip install -e ../test_utils
...
nox > pip install -e ../test_utils

Expected result (after the fix):
test_utils are only installed once in a session.

@plamut plamut added api: bigquery Issues related to the BigQuery API. type: cleanup An internal cleanup or hygiene concern. labels Oct 20, 2019
@plamut plamut requested a review from a team October 20, 2019 07:07
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 20, 2019
bigquery/noxfile.py Outdated Show resolved Hide resolved
bigquery/noxfile.py Outdated Show resolved Hide resolved
@tseaver
Copy link
Contributor

tseaver commented Oct 22, 2019

I would argue that dropping test_utils from LOCAL_DEPS would be clearer, if there are segments which don't actually need them.

@plamut
Copy link
Contributor Author

plamut commented Oct 22, 2019

@tseaver test_utils were actually moved to LOCAL_DEPS based on an earlier suggestion. :) All test suites need them (unit, system, snippets), although they seem redundant for the lint and docs nox sessions.

I can demote test_utils from LOCAL_DEPS to avoid that redundancy.

@plamut plamut requested a review from tseaver October 22, 2019 19:57
@plamut plamut added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 22, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 22, 2019
The test_utils dependency is only nedeed for test sessions, but not
for some other nox sessions such as "lint" and "docs".
@plamut
Copy link
Contributor Author

plamut commented Oct 24, 2019

The Cloud Build test failure is unrelated, currently occurring on all PRs, merging.

@plamut plamut merged commit 1f532f6 into googleapis:master Oct 24, 2019
@plamut plamut deleted the bq-noxfile-clean branch October 24, 2019 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BigQuery: Remove duplicate test dependencies from nox session configuration
5 participants