-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Moving all system test scripts in system_tests/. #1463
Conversation
Can we still exclude our system tests if they're a sub-package? If not, can we at least make the package private? |
Sure. Excluding them will mean they don't show up in an |
SGTM. Let's try that. On Fri, Feb 12, 2016, 11:26 PM Danny Hermes notifications@github.com
|
@jonparrott Had a revelation that I could just move those scripts into the |
bdbe005
to
aa5a498
Compare
Also removing system_tests/__init__.py so it is no longer a package and making all imports happen locally (rather than from the root of the project). Changes originally inspired by emulator script breakages in googleapis#1373.
aa5a498
to
8c1db22
Compare
I can't believe I didn't realize that either. LGTM. |
@@ -1,4 +1,3 @@ | |||
include README.rst | |||
graft gcloud | |||
global-exclude *.pyc | |||
recursive-exclude system_tests * |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Moving all system test scripts in system_tests/.
Also removing
system_tests/__init__.py
so it is no longer a package and making all imports happen locally (rather than from the root of the project).Changes originally inspired by emulator script breakages in #1373.
@jonparrott Having to worry about which scripts can see
system_tests
is somewhat of an argument for moving intogcloud.system_tests
(discussion in #1451). Happy to go that route here instead. WDYT?As-is, the current fix makes
pylint
angry, so at the very least we should discuss what to do about that.