Fix tests on fedora #556
Fix tests on fedora #556
Conversation
This prevents tests to pass if the build fails - since if this happens, the test will still continue and try to run the container. This commit adds a new test entry called build, which will fail and stop all depending tests from running. This will also cause tests to correctly return non-zero return code.
|
@jackorp PTAL as well. |
|
[test-all] |
| @@ -54,14 +54,6 @@ run_test_application() { | |||
| docker run --user=100001 --rm --cidfile=${cid_file} ${IMAGE_NAME}-testapp | |||
| } | |||
|
|
|||
| ct_check_testcase_result() { | |||
| # Negative test & zero exit status = ERROR. | ||
| # Tests with '-should-fail-' in their name should fail during a build, | ||
| # expecting non-zero exit status. | ||
| evaluate_build_result() { |
There was a problem hiding this comment.
Copy paste from s2i-python-container.
Think about to move it to 'test-lib.sh' in future. Just nit-pick
|
LGTM. I'd also like to raise one more thought. While the build failed, the tests didn't fail. Even though I'd expect the tests to be using the built container, IOW to fail at some point because they can't use the container, they used I presume previous build result. Would it make sense to clean up the container after the collections of test that need it finish? |
I have also thought about that, and it seemed really weird, why can the test pass using a container that failed to build. It showed the same behavior even locally, even tho I have previously run tests on this container locally, so it would make sense that it used previous results from the last passed container build. I can investigate this further, but I feel like it might be better to split it into different issue+PR not to make this too big, since I think that the major change here is the dependency update, not general test fixes - but that's up to you. |
Absolutely. I was trying to find whether it was just me or if there is a cause for investigation. I'll make an issue.
Agreed. More general test fixes can be left for other time. |
Great, u can assign me to the issue, since I have already poked around the issue. I'll take a look at it when I'll have some spare time. |
|
@SlouchyButton #557 now exists, but I don't have sufficient perms to edit those fields. |
|
[test-all] |
|
I will fix OpenShift 4 tests later on. Let's get merge it. The failed tests do not block this PR. |
Two issues are addressed, both mentioned in #554. When build fails, it will now handle the situation correctly and stop all depending tests from running while also causing the test to fail and return non-zero code. Code that is handling this is taken from s2i-python-container.
While debugging the code, I have encountered a duplicate function defined in the
runfile, despite already being present intest-lib.sh, so the local version was removed.Dependency
nio4rinpuma-test-appis bumped to the newest version to prevent build failing. This is related to #550.Fixes: #554