Skip to content

Testing CI #61

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

Merged
merged 11 commits into from
Mar 20, 2025
Merged

Testing CI #61

merged 11 commits into from
Mar 20, 2025

Conversation

atteggiani
Copy link
Collaborator

Fixes #56

Overview

Added integration tests to CI workflow.

The added jobs do the following:

  1. Set development (testing) sites
  2. Set up SSH keys for connection to site
  3. Create tmp dir on the site
  4. Copy repo to the tmp dir on the site
  5. Create micromamba environment using env_dev.yml
  6. Install replace_landsurface in the environment (using micromamba run ... for simplicity)
  7. Run tests (using micromamba run ... for simplicity)
  8. Clean up environment and tmp directory (yes, I use rm -rf ..., but the path of the temporary directory should be well defined and I can't see a better way to avoid it. Happy to discuss alternative solutions though.

HOST, HOST_DATA, SSH_KEY and USER secrets, as well as the MICROMAMBA_EXE variable have been set for the Gadi GitHub environment.

@atteggiani atteggiani requested a review from CodeGat March 3, 2025 22:35
@github-project-automation github-project-automation bot moved this to New Issues in ACCESS-rAM3 Mar 3, 2025
@atteggiani atteggiani marked this pull request as ready for review March 3, 2025 22:35
@atteggiani atteggiani marked this pull request as draft March 3, 2025 23:51
@atteggiani atteggiani force-pushed the davide/testing_ci branch 2 times, most recently from 72509f6 to de577bd Compare March 4, 2025 01:17
@atteggiani
Copy link
Collaborator Author

atteggiani commented Mar 13, 2025

Clean up environment and tmp directory (yes, I use rm -rf ..., but the path of the temporary directory should be well defined and I can't see a better way to avoid it. Happy to discuss alternative solutions though.

I've been using rm -rf quite a bit as well.. Though this does remind me of a forum post about safer alternatives to rm - https://forum.access-hive.org.au/t/safer-alternative-to-rm/4092. So I'm wondering in future that it might be worth having a designated trash folder for the service user that is cleaned up frequently so it gives a little time to recover if things got mistakingly deleted.

I like this idea and I think the suggestions raised in the post are very useful!
It will go beyond this PR, but we could think of having a similar setup for our service users? @CodeGat

jo-basevi
jo-basevi previously approved these changes Mar 19, 2025
Copy link
Collaborator

@jo-basevi jo-basevi left a comment

Choose a reason for hiding this comment

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

Added changes look good to me

… the running script fails or is interrupted, but not if the tests fail.
…rectory get removed in all cases when the script exits
jo-basevi
jo-basevi previously approved these changes Mar 20, 2025
Copy link
Collaborator

@jo-basevi jo-basevi left a comment

Choose a reason for hiding this comment

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

Thanks @atteggiani, looks good to me! Just a minor style comment but I think it'll work

Copy link
Collaborator

@jo-basevi jo-basevi left a comment

Choose a reason for hiding this comment

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

LGTM :)

@atteggiani atteggiani merged commit 03639d5 into main Mar 20, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this from New Issues to Done ✅ in ACCESS-rAM3 Mar 20, 2025
@atteggiani atteggiani deleted the davide/testing_ci branch March 20, 2025 05:47
@atteggiani
Copy link
Collaborator Author

Thank you @jo-basevi and @CodeGat!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done ✅
Development

Successfully merging this pull request may close these issues.

Add integration tests in the CI workflow
3 participants