-
Notifications
You must be signed in to change notification settings - Fork 328
Support PyTest container test suite for Python Container. #768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This blocks: sclorg#768 Signed-off-by: Petr "Stone" Hracek <[email protected]>
This blocks: #768 Signed-off-by: Petr "Stone" Hracek <[email protected]>
5866b10 to
cf7da90
Compare
|
Rebased against master. Let's run first round of tests. |
Testing Farm results
|
|
RHEL8 failures are caused by CDN errors: |
6a695bd to
e2004a5
Compare
|
[test][test-all] |
|
The new container-ci-suite was released. Let's re-test it. [test-all] |
|
Python failures are: C10S - PyTest - 3.12-minimal: It caused, I guess, main image should not be pulled. C10S - 3.12 should not be exists in quay.io/sclorg/python-312-c10s. RHEL10 does not exists. Fedora PyTest - 3.13 - test_container_basics.py::TestS2IPythonContainer::test_dockerfiles[Dockerfile.tpl] INFO:Image name to test: quay.io/fedora/python3-313:3.13 RHEL8 - 3.11 and RHEL9-PyTest-3.11 |
I don't understand this sentence, sorry. We have dockerfiles here for 3.12 and 3.12-minimal based on C10S.
The version of psycopg2-binary I see in the logs indicates that this test used django-ex branch 3.2.x and that's incorrect. New Pythons like 3.13 should use Django version/branch 4.2.x that has newer psycopg2-binary dependency and that new version provides wheels for Python 3.13.
I need more time to investigate the last one. I'm not able to reproduce locally so I need to build the container image first. |
Also VARS.* variables are used during the migration. Signed-off-by: Petr "Stone" Hracek <[email protected]>
Migration matrix is: `build_s2i_app` is fixture that takes `Path` test-app from `conftest.py` file. test_container_application.py tests all test-app directories. test_container_basics.py tests only basics w/o application. It also tests test_dockerfile and test_minimal_dockerfile Signed-off-by: Petr "Stone" Hracek <[email protected]>
Signed-off-by: Petr "Stone" Hracek <[email protected]>
Add distgen generated files Signed-off-by: Petr "Stone" Hracek <[email protected]>
Fix proper branch in `test_dockerfiles` function. Add test suite to Class and build application once. Signed-off-by: Petr "Stone" Hracek <[email protected]>
e2004a5 to
df1b3e2
Compare
|
I have fixed some issues. Let's re-run tests again |
frenzymadness
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good overall. Just a few nitpicks.
| image_name=full_image_name, loops=1 | ||
| ) | ||
| print(f"Is image {full_image_name} pulled? {is_pulled}") | ||
| if is_pulled: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this part just fail when we are unable to pull the full image and therefore run the tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See here: https://github.com/sclorg/s2i-python-container/blob/master/test/run#L236. In case it is not pull, then skip the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem in bash suite is here:
$ IMAGE_NAME=quay.io/sclorg/python-39-minimal-c9s:39-minimal
$ NEW_IMAGE=${IMAGE_NAME/-minimal/}
$ echo $NEW_IMAGE
quay.io/sclorg/python-39-c9s:39-minimal
$
which is totally different image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frenzymadness What about if full version does not exist? Like python-312-minimal for RHEL10? It should not fail, I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation, that makes sense. Could you please improve the message to something like: The full container image for the minimal one does not exist, skipping the rest of the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by 4638046
|
Let me know if you want me to take a look at the results of the failed tests. |
Use default branch 4.2.X for all versions except 3.6 Signed-off-by: Petr "Stone" Hracek <[email protected]>
|
Let's test it again [test-pytest] |
minimal one does not have it. Signed-off-by: Petr "Stone" Hracek <[email protected]>
|
[test-pytest] |
This pull request migrates bash test suite
to container-ci-suite stored here: https://github.org/sclorg/container-ci-suite
Introduce PyTest suite migrated from test/run bash suite.
Migration matrix is:
build_s2i_appis fixture that takesPathtest-app fromconftest.pyfile.
test_container_application.py tests all test-app directories.
test_container_basics.py tests only basics w/o application.
It also tests test_dockerfile and test_minimal_dockerfile
Signed-off-by: Petr "Stone" Hracek [email protected]