ci: add docker image test workflow #332
Conversation
marcalexiei
changed the title
There was a problem hiding this comment.
Thank you for the initial diving on testing docker.
I’ve looked into using WSL and how to install it through a GitHub Actions workflow, but I haven’t had success so far. I’ll try again during the week.
Consider to turn the PR into draft if you are still working on it.
As I write in #295 (comment), I'm fine by testing only linux env if windows and Mac turn out to so hard to work with.
.github/workflows/docker.yml
Outdated
There was a problem hiding this comment.
Could you please rename the file into docker-ci.yml?
.github/workflows/docker.yml
Outdated
| on: | ||
| push: | ||
| branches: | ||
| - 'chore-add-image-build-test-to-github-actions' |
There was a problem hiding this comment.
on needs refine otherwise it will never run 😄
| on: | |
| push: | |
| branches: | |
| - 'chore-add-image-build-test-to-github-actions' | |
| on: | |
| push: | |
| branches: | |
| - 'main' | |
| pull_request: |
.github/workflows/docker.yml
Outdated
| matrix: | ||
| os: | ||
| - ubuntu-latest | ||
| - macos-13 # on intel chipset |
There was a problem hiding this comment.
I managed to make it work, but the result is not very satisfying because it only works on macOS version 13. I am using the Colima container runtime, which adds a significant layer to the process and thus impacts the build time on GitHub Actions.
I see on your fork that the build takes more than 30minutes to succeed so I would rather comment macOS from the os matrix.
.github/workflows/docker.yml
Outdated
| ln -sfn $(brew --prefix)/opt/docker-compose/bin/docker-compose ~/.docker/cli-plugins/docker-compose | ||
| colima start | ||
| - name: Build and Run Docker Compose on linux/windows |
There was a problem hiding this comment.
| - name: Build and Run Docker Compose on linux/windows | |
| - name: Build and Run Docker Compose |
.github/workflows/docker.yml
Outdated
| - name: Check commands inside container | ||
| run: | | ||
| docker exec tuono-source-container pnpm --version || { echo "Error: pnpm is not available."; exit 1; } | ||
| docker exec tuono-source-container tuono --version || { echo "Error: tuono is not available."; exit 1; } |
There was a problem hiding this comment.
How about adding also a run of both pnpm and cargo tests?
There was a problem hiding this comment.
Hmmm good point !
This way, we could potentially catch issues that depend on the OS. However, this would come at the cost of a longer CI execution time for Docker.
I'll share a comparison of the two execution times here to help decide on the matter.
There was a problem hiding this comment.
As you can see here, the test steps ("run cargo test" and "run pnpm test") take a relatively short amount of time to complete.
Additionally, the "run pnpm test" step executes all existing tests in the packages directory. This setup ensures that if we add new tests, for instance, in the /packages/tuono directory in the future, they will automatically be included in the "pnpm" test step.
I didn't have time this week to explore running the CI on Windows or macOS. I'm unsure which OS to prioritize—macOS might be a bit more challenging for me to set up. However, feel free to share your preferences on this! :)
We could stop here for a first version, but I find it a bit disappointing to manage only one OS with Docker. Let me know what you think! :D
There was a problem hiding this comment.
As you can see here, the test steps ("run cargo test" and "run pnpm test") take a relatively short amount of time to complete.
Excellent! It's great to have them working.
I didn't have time this week to explore running the CI on Windows or macOS. I'm unsure which OS to prioritize—macOS might be a bit more challenging for me to set up. However, feel free to share your preferences on this! :)
Every little help we can get is very welcome, so go ahead with the one you are more comfortable!
We could stop here for a first version,
I agree, we can merge this PR with only ubuntu os working.
Please merge main branch changes and ensure that CI is passing,
Once CI is green request a new review from me 😉
but I find it a bit disappointing to manage only one OS with Docker. Let me know what you think! :D
I understand your point.
If you want to keep investigate this further for additional OSes,
I can create new sub tasks (thanks GitHub Issues public preview 🤣)
in the linked issue to handle Windows and MacOS docker tests
spacecodeur
force-pushed
the
chore-add-image-build-test-to-github-actions
branch
from
03a16bd to
be374b2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
spacecodeur
force-pushed
the
chore-add-image-build-test-to-github-actions
branch
4 times, most recently
from
e50c891 to
569f05d
Compare
spacecodeur
force-pushed
the
chore-add-image-build-test-to-github-actions
branch
from
569f05d to
7e18628
Compare
Context & Description
The purpose of this PR is to propose an improvement to the initial version of the dockerization of the "source code" repository of Tuono: this improvement adds the configuration file for GitHub Actions.
An issue has been created here following the merge of the PR that included the Docker configuration..
Here is the current state I have reached so far:
Linux job: It works very well ! :D
Mac OS job:
Windows job:
It seems the current base image used in Dockerfile (docker/Dockerfile : rust:bookworm) that the official Rust image does not support Windows as host machine.
If you’d like, I can start opening a PR for this topic. I will likely need help from Mac users to find the right setup for the GitHub Actions workflow.