Create tox environment for running tests #144
Conversation
| deps = | ||
| absl-py | ||
| google-api-core | ||
| google-cloud-spanner >= 1.17.1 | ||
| frozendict | ||
| portpicker |
There was a problem hiding this comment.
Is there any way to keep this in sync with other places? E.g. could we put this in a requirements.txt or something, and reference that here?
There was a problem hiding this comment.
While these can be put in a requirements.txt file, I don't think setup.py should be referencing that file, so there still would be duplication
https://stackoverflow.com/a/33685899
There was a problem hiding this comment.
I ended up adding the below in a different PR because of the same issue with setup.py not referencing requirements.txt. Would it make sense to add requirements.txt here, and use it both for tox and the github workflow? Or would the github workflow use tox and not need to reference the dependencies explicitly anymore?
tox.ini
Outdated
| wget https://storage.googleapis.com/cloud-spanner-emulator/releases/{env:VERSION}/cloud-spanner-emulator_linux_amd64-{env:VERSION}.tar.gz -P /tmp/spanner_emulator/ | ||
| tar zxvf /tmp/cloud-spanner-emulator_linux_amd64-{env:VERSION}.tar.gz -C /tmp/spanner_emulator/ | ||
| chmod u+x /tmp/spanner_emulator/gateway_main /tmp/spanner_emulator/emulator_main | ||
| python setup.py test |
There was a problem hiding this comment.
The PR description mentions a warning about doing this. Should this be something else?
There was a problem hiding this comment.
I think doing that in a tox env is still OK, but just to be safe, I switched to pytest. It has a cleaner test output too!
tox.ini
Outdated
| wget https://storage.googleapis.com/cloud-spanner-emulator/releases/{env:VERSION}/cloud-spanner-emulator_linux_amd64-{env:VERSION}.tar.gz -P /tmp/spanner_emulator/ | ||
| tar zxvf /tmp/cloud-spanner-emulator_linux_amd64-{env:VERSION}.tar.gz -C /tmp/spanner_emulator/ | ||
| chmod u+x /tmp/spanner_emulator/gateway_main /tmp/spanner_emulator/emulator_main |
There was a problem hiding this comment.
Is this going to run every time we run tests? Is there any way to cache it? (Feel free to push this off to another PR.)
There was a problem hiding this comment.
Yeah, I'm inclined to push this off for now. In practice (on my admittedly fast internet connection) this steps only takes a couple of seconds.
| cloud-spanner-emulator* | ||
| emulator_main | ||
| gateway_main | ||
| .spanner_emulator/** |
There was a problem hiding this comment.
(nit) add / at the beginning of the line, so it only matches in this directory.
dseomn
left a comment
dseomn
left a comment
There was a problem hiding this comment.
Mind updating https://github.com/google/python-spanner-orm/blob/main/.github/workflows/test.yaml to use tox too?
| deps = | ||
| absl-py | ||
| google-api-core | ||
| google-cloud-spanner >= 1.17.1 | ||
| frozendict | ||
| portpicker |
There was a problem hiding this comment.
I ended up adding the below in a different PR because of the same issue with setup.py not referencing requirements.txt. Would it make sense to add requirements.txt here, and use it both for tox and the github workflow? Or would the github workflow use tox and not need to reference the dependencies explicitly anymore?
| absl-py | ||
| google-api-core | ||
| google-cloud-spanner >= 1.17.1 | ||
| frozendict |
| deps = | ||
| absl-py | ||
| google-api-core | ||
| google-cloud-spanner >= 1.17.1 |
There was a problem hiding this comment.
(nit) This doesn't match the other places, but I don't know what it should be.
| # and then run "tox" from this directory. | ||
|
|
||
| [tox] | ||
| envlist = py36, py37, py38 |
2 participants
When running
python setup.py testin the past, we've gotten the following messageWARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox.. This PR listens to that advice and creates a tox environment for running tests :)The nice thing about using tox for tests is we can integrate the setup of Spanner emulator into the tox command.
After a couple people confirm that tox runs for them successfully (i.e. by running
tox -e py38), I will update theREADMEto point users to run tests with tox.Tested: ran tests successfully.