Make changes to CI so Docker image is reused. #110
Conversation
haydenroche5
force-pushed
the
reuse_docker_image
branch
17 times, most recently
from
6d998f0 to
5b7aa0d
Compare
Dockerfile
Outdated
| @@ -83,7 +89,6 @@ RUN ["dash", "-c", "\ | |||
| "] | |||
|
|
|||
| # Configure Arduino CLI | |||
| USER ${USER} | |||
There was a problem hiding this comment.
This line was what was causing all the failed builds while I was iterating on this PR. See https://docs.github.com/en/actions/creating-actions/dockerfile-support-for-github-actions#user.
There was a problem hiding this comment.
It's important to have this. Otherwise, root owns everything you build on your machine when you run the container locally. Then you don't have permissions to delete build artifacts, when you are outside the container.
test/run_all_tests.sh
Outdated
| @@ -164,20 +168,21 @@ if [ 0 -eq ${all_tests_result} ]; then | |||
|
|
|||
| # Run coverage if available | |||
| if [ $(which lcov) ]; then | |||
| rm mock-*.gc?? *_Mock.gc?? *test.gc?? \ | |||
| && gcov --version \ | |||
| rm mock-*.gc?? *_Mock.gc?? *test.gc?? | |||
There was a problem hiding this comment.
I was failing here locally and that was causing the rest of the coverage commands below to not happen. I separated out this rm step from the rest of the steps below so that if the rm fails, we still do the other steps.
There was a problem hiding this comment.
Good catch!
I think we can just add the -f flag to the rm to ensure rm doesn't "fail".
I agree it's probably better to run on it's own, because running coverage should not be dependent on it's success. Let's go ahead and add the -f parameter, because it will clean up the logs.
| args: | ||
| - -c | ||
| - "cp -r . /home/blues/Arduino/libraries/Blues_Wireless_Notecard \ | ||
| && HOME=/home/blues arduino-cli compile \ |
There was a problem hiding this comment.
This is the part you missed, and why you had to comment out the USER instruction from the Dockerfile (actions/runner#1876).
Dockerfile
Outdated
| @@ -83,7 +89,6 @@ RUN ["dash", "-c", "\ | |||
| "] | |||
|
|
|||
| # Configure Arduino CLI | |||
| USER ${USER} | |||
test/run_all_tests.sh
Outdated
| @@ -164,20 +168,21 @@ if [ 0 -eq ${all_tests_result} ]; then | |||
|
|
|||
| # Run coverage if available | |||
| if [ $(which lcov) ]; then | |||
| rm mock-*.gc?? *_Mock.gc?? *test.gc?? \ | |||
| && gcov --version \ | |||
| rm mock-*.gc?? *_Mock.gc?? *test.gc?? | |||
There was a problem hiding this comment.
Good catch!
I think we can just add the -f flag to the rm to ensure rm doesn't "fail".
I agree it's probably better to run on it's own, because running coverage should not be dependent on it's success. Let's go ahead and add the -f parameter, because it will clean up the logs.
Dockerfile
Outdated
| @@ -83,7 +89,6 @@ RUN ["dash", "-c", "\ | |||
| "] | |||
|
|
|||
| # Configure Arduino CLI | |||
| USER ${USER} | |||
There was a problem hiding this comment.
It's important to have this. Otherwise, root owns everything you build on your machine when you run the container locally. Then you don't have permissions to delete build artifacts, when you are outside the container.
|
Did coverage drop because we are no longer running the It's probably more accurate, but let's figure out what happened. |
haydenroche5
force-pushed
the
reuse_docker_image
branch
from
3077cf4 to
7c0483e
Compare
haydenroche5
force-pushed
the
reuse_docker_image
branch
from
7c0483e to
a50c1d4
Compare
No description provided.