-
Notifications
You must be signed in to change notification settings - Fork 3
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
[Testing] Test SSH file transfer with images. #208
base: main
Are you sure you want to change the base?
Conversation
7f9781b
to
7d6add8
Compare
8d0f0a3
to
9442694
Compare
88591cb
to
bafb5d0
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
d503958
to
b4bb0a4
Compare
611e63e
to
3bae225
Compare
3bae225
to
689db1d
Compare
acb1d2b
to
085ef63
Compare
b49073e
to
8f4d341
Compare
085397d
to
ddb81d5
Compare
…t/datashuttle into test_ssh_with_image
208d049
to
f47884c
Compare
f47884c
to
b26163b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My intermediate report on this:
- The ssh tests failed when locally running on MacOS (M2), with docker running
- The contributing guide should be updated to let contributors know how to handle local ssh tests (either skip them or configure their local machines accordingly).
sudo service mysql stop # free up port 3306 for ssh tests: https://github.com/orgs/community/discussions/25550 | ||
pytest | ||
else | ||
pytest -k "not test_combinations_ssh_transfer and not test_ssh_setup" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this mean that on other OSes pytest will run all tests except the ones specified here? That's what we'd want/expect right?
This PR implements SSH tests through a docker image. They key changes in this PR are:
code_test_and_deploy.yml
to selectively run SSH tests in linux runners. This is because the Windows and macOS runners already are in a container, so it's not possible to run a container from within these containers ('nested containerisation').canonical_configs.py
. At the moment this is not documented / exposed to users. In general I don't think there will be much need to SSH from other ports except for 22. But it's possible it will be needed.Dockerfile
is added that spins up an ubuntu image with SSH setup so that it can be SSHed into from tests.ssh_test_utils.py
now contains all machinery to SSH into the docker image.The rest of this PR is pretty much refactoring.
base_transfer.py
,test_ssh_file_transfer.py
,test_ssh_setup.py
just contains the already-existing tests. The new additions is testing the SSH through images not through a configuration where paths etc. were entered into theconftest.py
. However, any feedback on these tests is of course welcome.