extract the integration tests writen in the ci file as a script file. #37
Conversation
|
It's not a problem in CI, but in my local, the memory of cgroups failed. |
|
I'm wondering if it might be worthwhile to setup local tooling for testing against multiple different Linux distributions. This work might actually help with this. I'll outline that work in a new ticket. |
There was a problem hiding this comment.
Unless I'm missing something, I don't think this works since the integration tests are never build before executing. Probably adding a short instruction to the README.md would be a good idea. Also probably need to outline that in order to run the tests locally you need to have golang compiler and node-tap dependencies installed.
| exit 1 | ||
| fi | ||
| done | ||
| run: ./integration_test.sh |
There was a problem hiding this comment.
Where does the build happen for the integration tests now?
There was a problem hiding this comment.
@tsturzl The build of the integration test is included in this script.
integration_test.sh
Outdated
| @@ -0,0 +1,12 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
This script fails locally for me because the tests haven't been compiled. Also setting up testing might be worth a short addition to the README.md
There was a problem hiding this comment.
So the integration test passing was actually a false positive, if you look at it the script failed in CI also. I think the script itself must not be returning the exit code correctly. So maybe this is another thing to fix, making sure the integration tests fail if anything returns a non-zero code, perhaps this is as easy as adding set -e to the script? See the CI failure here: https://github.com/utam0k/youki/pull/37/checks?check_run_id=2673869152#step:6:18
|
@tsturzl |
utam0k
force-pushed
the
add-integration-tests
branch
5 times, most recently
from
742335a to
1db813a
Compare
utam0k
force-pushed
the
add-integration-tests
branch
from
1db813a to
461fae0
Compare
|
@tsturzl |
There was a problem hiding this comment.
Actually, take a look at the CI. The CI is still failing, which might not be an issue concerning this ticket, however the CI on this PR says it succeeded when it really didn't. The exit code of the script is being set explicitly instead of returning the error code of the failed command. If you add set -e to the top of integration_test.sh that should fix the issue and it should error out of the bash script with the proper exit code. I believe using set -e will give a similar behavior to when the commands were written inline in the workflow yaml, where the script fill fail and return the failure exit code whenever any command comes back non-zero.
This reverts commit d3ad7c6.
|
@tsturzl Thank you for being so cautious. Your review is very helpful. |
|
@utam0k I agree, it looks like as far as this PR is concerned things look good. However I'm going to file a bug report concerning this failure hopefully tonight after dinner when I have time to read through it a little more. |
|
@tsturzl
|
|
I think I totally misunderstood you before. I thought the error messages were failures and the exit codes were maybe getting caught somewhere. My mistake. I don't believe there to be a bug, I just missed what you said:
Sometimes I'm reading these on my phone while I'm doing other things, and miss things. Thank you for the clarification! |
|
@tsturzl In fact, your point wasn't wrong, since the CI passed even if the first commit failed. |
No description provided.