Closed rgschmitz1 closed 2 years ago
As an alternative, this whole test could be condensed to a single line using a ternary operator and the shell built in true/false
RUN $TEST_ENABLED && eval "$TEST_COMMAND" || echo "skipping tests"
Instead of this.
RUN if [ "$TEST_ENABLED" == "false" ]; then \
echo "skipping tests";\
else \
eval "$TEST_COMMAND"; \
fi
@rgschmitz1 I think changing to a single =
make sense
I am not sure about the ternary, would it hide errors when eval fails and just skip testing when the eval fails?
RUN $TEST_ENABLED && eval "$TEST_COMMAND" || echo "skipping tests"
I think the desired behavior should be a. if explicitly skipped, print the message b. if not skipped, eval the test command and fail if it returns and error
Hi Lucas, thanks for the reply.
I think the desired behavior should be a. if explicitly skipped, print the message b. if not skipped, eval the test command and fail if it returns and error
If I understand correctly, if eval "$TEST_COMMAND"
returns an error status, this should cause the container build to exit with a error.
I definitely don't want to introduce anything that would cause issues down the road for the sake of more concise code. I can double check the ternary behavior in alpine and debian images and report back here with a gist of the results to have a full record. I'll check the results for both proposed solutions and just leave it up to you about the final patch.
@rgschmitz1 i have no complaints with the ternary, as long as it works :) I look forward to your results. It will be good to have a consistent and correct implementation for debian and alpine.
@LucasRoesler
It's been a busy week, I just managed to get to testing and generating logs for ternary. Ternary would work however not exactly how I proposed originally. Turns out the eval "$TEST_COMMAND"
portion needs to come last or any failures would be masked by the echo "skipping test"
command as you mentioned might be the case (shell scripts are weird).
If a ternary is implemented as follows then there didn't appear to be any issues.
[ "$TEST_ENABLED" = "false" ] && echo "skipping test" || eval "$TEST_COMMAND"
Here's a log of results and dockerfiles used to test, https://gist.github.com/rgschmitz1/1cc3f375e582433dc80191b87d2e103e
In any case just putting a single = in the conditional would work also.
Hi @rgschmitz1 looks good. I am happy with either version. The ternary seems to be pretty readable, especially in the Dockerfile.
Do you want to patch it?
Hi @LucasRoesler , I was just waiting for a formal "all good to patch" message. 👍🏻
I'll submit a pull request today, thanks!
My actions before raising this issue
faas build fails with tox error #44 seems like it may be related
Expected Behaviour
Dockerfiles using
python:3.7-slim-buster
base image should skip testing when TEST_ENABLED is specified as false.Current Behaviour
The TEST_ENABLED check is throwing a syntax error due to syntax inconsistencies between alpine (busybox) and debian (dash) implementations of the default shell. This doesn't allow the use of bypassing the test using the
--build-arg "TEST_ENABLED=false"
flag as suggested in the documentation.Possible Solution
I suggest replacing the double equal (==) in the Dockerfile templates with a single equal (=) to be more POSIX friendly, this worked for me in both alpine and debian versions of the python image.
Steps to Reproduce (for bugs)
faas-cli template pull https://github.com/openfaas-incubator/python-flask-template
faas-cli new --lang python3-http-debian hello-python
faas-cli build -f hello-python.yml
Context
I'm a college student working on a capstone project involving deploying open-source cloud native applications in Kubernetes and doing a comparison against vendor specific solutions.
I'm using of-watchdog templates as a basis for my own custom functions and would love to contribute in any way I can.
Your Environment
faas-cli version
):CLI: commit: b1c09c0243f69990b6c81a17d7337f0fd23e7542 version: 0.14.2
Docker version 20.10.14, build a224086
NAME="Ubuntu" VERSION="20.04.4 LTS (Focal Fossa)" ID=ubuntu ID_LIKE=debian PRETTY_NAME="Ubuntu 20.04.4 LTS" VERSION_ID="20.04" HOME_URL="https://www.ubuntu.com/" SUPPORT_URL="https://help.ubuntu.com/" BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/" PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy" VERSION_CODENAME=focal UBUNTU_CODENAME=focal