Python function test

[jrvidotti]

My actions before raising this issue

• Followed the troubleshooting guide
• Read/searched the docs
• Searched past issues

Expected Behaviour

Using python3-http template, there is a file handler_test.py. Modified it to not pass the test:

from .handler import handle

def test_handle():
    assert False

I've built the function and I was expecting to get an test error. Did a local run and also got no test errors.

Current Behaviour

I didn't get any type of test errors.

Possible Solution

Steps to Reproduce (for bugs)

1. Write failing tests
2. Build and get no errors.

Context

Testing code is essential. Providing a solid way to write and run tests is a must.

Your Environment

• FaaS-CLI version ( Full output from: faas-cli version ):

CLI:
commit: f72db8de657001a2967582c535fe8351c259c5f6
version: 0.16.17

• Docker version docker version (e.g. Docker 17.0.05 ):
  Docker Community 24.0.7

• Are you using Docker Swarm or Kubernetes (FaaS-netes)?
  Kubernetes

• Operating System and version (e.g. Linux, Windows, MacOS):
  Linux Ubuntu

• Code example or link to GitHub repo or gist to reproduce problem:

• Other diagnostic information / logs from troubleshooting guide

Next steps

You may join Slack for community support.

[alexellis]

@LucasRoesler would you have any links etc that you can share on this for @vidz1979 ?

@vidz1979 please edit and fill out the whole issue template

https://raw.githubusercontent.com/openfaas/python-flask-template/master/.github/ISSUE_TEMPLATE.md

[LucasRoesler]

@alexellis i just tested this, it is a bug, I can replicate it and the cause was this 79b444b

FROM build as test
ARG TEST_COMMAND=tox
ARG TEST_ENABLED=true
RUN [ "$TEST_ENABLED" = "false" ] && echo "skipping tests" || eval "$TEST_COMMAND"

We (me) moved the test command into a separate Docker stage, but because the final stage doesn't depend on it, it is pruned from the build and never executes.

If you, for example, force ship to depend on test, it will force the test stage to actually run

FROM test as ship

instead of

FROM build as ship

I verified that this works.

[LucasRoesler]

I was thinking about this a bit. I am not sure what the best option is here. If the goal is a small final image, running tests is going to work against us, because the env will need all dev time dependencies.

one option that we could use, is to install everything in the build layer. In fact, i woudl install it twice, one generally and then the runtime only dependencies into a virtualenv folder. Then in the final ship layer only needs to copy the virtualenv folder to the final layer and modify the path. I have been using this technique successful in many python images recently and it works very well.

[jrvidotti]

I confirm that doing FROM test as ship works!

[ccfontes]

Running into the similar issue using the FROM build as test tactic to reduce final image size (inspired by this Python template) in faas-bb template.

I find it intriguing we're running into this issue more recently although the apparent root cause is old - Docker multi stage build ignoring intermediate stages, caused by DOCKER_BUILDKIT. Folks in the ticket mention DOCKER_BUILDKIT=0 fixing the issue, but I could see this being a problem to support multiple architectures.

From what I understand @LucasRoesler proposes a python specific sandboxing solution. If we could come up with a more generic solution, folks in other langs for openfaas could benefit too.

[ccfontes]

Just chiming in that DOCKER_BUILDKIT=0 faas build .. did the trick of not ignoring the orphan intermediate stage. Should also work with this template. For now going with this solution. Hope it helps.

[LucasRoesler]

In terms of generic solutions. Obviously the "disable buildkit" works for now, but you can imagine this might eventually be deprecated, I am pretty sure it is already the default builder for most installations now. I hate depending on this or even needing to tell every new user that they have to disable a defaultndocker behavior to get th expected behavior.

For the solution to be fully generic for any language and avoid including the test later in the final image, it means we have to do something via docker itself. The only approach I see is to allow passing a build target to the --target flag so that users can explicitly run the build layer. Each template would need to document how to run tests, if possible.

Every other solution I can think of would be highly language specific.

[ccfontes]

I ended up going for a language specific solution to not ship test related assets, for the same reason I didn't think it was good UX having to pass (and document) DOCKER_BUILDKIT=0. Also as you mentioned, might get deprecated as option. Also unmentioned here that having buildkit enabled has other advantages like running multiple stage builds in parallel from what I read briefly.

I'll have a look into using build target and implications to UX. Thanks.

[LucasRoesler]

@ccfontes if we get some input from @alexellis i would be happy to add a new flag to the faas-cli build or refactor this image so that it runs the tests in a non-orphaned layer.

The python specific alternative that i can think of is to install the whole env in the build layer, run the tests, then install the env again into a venv folder. It is quite easy to move the venv to the final layer and adjust the PATH variable to enable it. This is a bigger change and wouldn't really do much for this specific template/image because we don't support the concept of dev or test dependencies, it is a single requirements.txt, so the build and final layers have the same dependencies regardless. The venv approach would be easier with a tool like Hatch or Poetry or if we documented a flow with requirements-dev.txt (or similar)

[alexellis]

Does it makes sense to go with this solution?

#71 (comment)

[ccfontes]

@alexellis If you have an idea why it makes sense or doesn't make sense to go with #71 (comment) solution, it would be helpful to state that idea, would you agree?

[alexellis]

I'm not trying to get too involved here, just suggesting that we go with Lucas' solution. The only one he mentioned in the linked comment.

Do you all agree, or do you want something else?

[ccfontes]

I misread, and very sorry for the misunderstanding. 🙏

I'm OK with @LucasRoesler's solution. Doing the same in faas-bb dockerfile (kind of). It makes a language specific cleanup of the test deps.

[LucasRoesler]

@ccfontes and @alexellis i has pushed a PR #73