Reworked app.run exit code #73
Conversation
scif/tests/test_exec.sh
Outdated
| @@ -6,3 +6,4 @@ echo "Testing exec of commands:" | |||
| docker run -it "${CONTAINER_NAME}" exec hello-world-echo echo "Hello World!" | |||
| docker run -it "${CONTAINER_NAME}" exec hello-world-script ls / | |||
| docker run -it "${CONTAINER_NAME}" exec hello-world-env env | |||
| docker run -it "${CONTAINER_NAME}" exec hello-world-env echo [e]OMG | |||
There was a problem hiding this comment.
I'm not familiar with the test suite here, but does somewhere check that we get the expected TACOS from this command... or is it just ensuring the command runs and exits with success?
There was a problem hiding this comment.
I only followed the same pattern that existed, but had the same question. It is currently, I believe, just exiting with success (and i verified the output with my eyeballs). I will see if I can add a better check to that. I was in the process of trying to make my tests better and similar to those in test_tests.sh, but I'm not sure those are working properly as it is.
@vsoch how do you/CI run these tests?
|
This looks good to me - a very simple change to get the desired functionality, and with a test. @dtrudg do you approve as well? |
|
Alright - time for a take 2! |
Here is a new version of the same app.run exit code work. I added a new app to the hello-world scif and used it for tests in test_run. I also added a test for shell substitution issue that @dtrudg reported from my previous PR. This patch includes updates to the changelog and the scif/version.py to bump the version 0.0.84.