Skip to content
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

Sanity tests use Python version, they should not do that #120

Open
felixfontein opened this issue Aug 4, 2023 · 9 comments
Open

Sanity tests use Python version, they should not do that #120

felixfontein opened this issue Aug 4, 2023 · 9 comments

Comments

@felixfontein
Copy link

Sanity tests should not be run for specific Python versions. They should use --docker (for the default image) to run all sanity tests in a standarized environment.

See for example https://docs.ansible.com/ansible/devel/community/collection_contributors/collection_requirements.html#ci-testing, which explicitly states

Collections MUST run an equivalent of the ansible-test sanity --docker command.
If they do not use --docker, they must make sure that all tests run, in particular the compile and import tests (which should run for all supported Python versions).

Considering that collections can and should explicitly avoid certain Python versions in https://github.com/ansible-network/github_actions/blob/main/.github/workflows/sanity.yml, this doesn't seem to be satisfied.

@felixfontein
Copy link
Author

The correct way to run sanity tests is to pick one Python version for every ansible-core/ansible-base/Ansible version, use that Python version to install ansible-core, and run ansible-test sanity --docker.

@sean-m-sullivan
Copy link

@felixfontein I think the confusion on your pick one comes from this page.
https://docs.ansible.com/ansible/latest/reference_appendices/release_and_maintenance.html#ansible-core-support-matrix

It lists a RANGE of python for each version of Ansible, and for things that want to test the widest variety of support with the sanity plugin. Only choosing one:one doesn't seem to give wide coverage. However the docker solution is also interesting, but I don't think there is any documentation on WHY you want to use the docker version, and/or python version, and different ansible versions, and how best to do that. I'd love to pick your brain on what we should do.

I even just tried running the docker with python version and got

WARNING: There following sanity test virtual environments are out-of-date in the "default" container: sanity.changelog-3.9, sanity.runtime-metadata-3.9, sanity.yamllint-3.9
WARNING: Reviewing previous 1 warning(s):
WARNING: There following sanity test virtual environments are out-of-date in the "default" container: sanity.changelog-3.9, sanity.runtime-metadata-3.9, sanity.yamllint-3.9

@felixfontein
Copy link
Author

@sean-m-sullivan ansible-test sanity --docker -v does not chose one Python version, it chooses all supported (by both ansible-core and the collection, as determined by tests/config.yml) for the tests where using more than one version makes sense (mainly compile and import). All other tests run with one version of Python. You should never use --python with ansible-test sanity --docker except to run a specific subset of tests.

There is a reason why the CI of ansible/ansible has always (or at least for the last four+ years) been using ansible-test sanity --docker without --python (see https://github.com/ansible/ansible/blob/devel/.azure-pipelines/commands/sanity.sh#L39-L41). (It only uses --test and --skip-test to split up all sanity tests into two groups, instead of running everything at once.)

@sean-m-sullivan
Copy link

okay, do see a collection with this set,
https://github.com/ansible/awx/blob/devel/awx_collection/tests/config.yml#L1C1-L3C24

But I am looking now, under the docs, looking at varous places, and not seeing anything about a config file or what should be in it. Is there somewhere I should be looking?

So we should install a version of ansible, and then run ansible-test sanity --docker -v

Maybe we should have a way of running sanity docker that has runs different versions of ansible and then runs tests like you say with different versions of python?

just trying to understand this, so I can upgrade my testing

@felixfontein
Copy link
Author

The config file is documented here: https://github.com/ansible/ansible/blob/devel/test/lib/ansible_test/config/config.yml
I personally always add a link to that in the config file itself (as in https://github.com/ansible-collections/community.general/blob/main/tests/config.yml#L7). The only place I know in the documentation where this is mentioned is https://docs.ansible.com/ansible/devel/dev_guide/testing/sanity/compile.html#compile - see the "Note" at the bottom.

So we should install a version of ansible, and then run ansible-test sanity --docker -v

What you usually do is install all versions of ansible-core that the collection supports, and for every one run ansible-test sanity --docker -v. This is for example what the example CI included in the collection_template does: https://github.com/ansible-collections/collection_template/blob/main/.github/workflows/ansible-test.yml#L62-L111

So basically the matrix for sanity tests should only contain a list of ansible-core versions resp. branches (stable-2.x/devel/milestone).

@sean-m-sullivan
Copy link

Thanks this makes sense, I am thinking some of this should be in an ansible-doc page, so people can find it easier, but knowing about the docker option checking multiple versions of python, and the collection template, points me in the right direction.

@felixfontein
Copy link
Author

I agree, this isn't really (well) documented at the moment.

@cidrblock
Copy link
Member

cidrblock commented Aug 30, 2023

I have a preference for this approach, each combination of python version and core version is reflected for unit, integration and sanity. I find this easier to navigate, isolate errors, and reproduce locally.

Unless I'm missing something here, I think this provides the same coverage as a single ansible-test sanity --docker ?

https://github.com/ansible-collections/ansible.scm/actions/runs/6016182974

also- using a combination of the ansible extension and the tox-ansible plugin, each entry in the matrix shows in the vsCode test tree and allows for point and click running etc.

(the tox functionality is still in preview and a little buggy)

image

The same is avaiable at the command line

tox list --ansible -c tox-ansible.ini
default environments:
integration-py3.8-2.9        -> Integration tests using ansible 2.9 and python 3.8
integration-py3.8-2.12       -> Integration tests using ansible-core 2.12 and python 3.8
integration-py3.8-2.13       -> Integration tests using ansible-core 2.13 and python 3.8
integration-py3.9-2.12       -> Integration tests using ansible-core 2.12 and python 3.9
integration-py3.9-2.13       -> Integration tests using ansible-core 2.13 and python 3.9
integration-py3.9-2.14       -> Integration tests using ansible-core 2.14 and python 3.9
integration-py3.9-2.15       -> Integration tests using ansible-core 2.15 and python 3.9
integration-py3.10-2.12      -> Integration tests using ansible-core 2.12 and python 3.10
integration-py3.10-2.13      -> Integration tests using ansible-core 2.13 and python 3.10
integration-py3.10-2.14      -> Integration tests using ansible-core 2.14 and python 3.10
integration-py3.10-2.15      -> Integration tests using ansible-core 2.15 and python 3.10
integration-py3.10-devel     -> Integration tests using ansible-core devel and python 3.10
integration-py3.10-milestone -> Integration tests using ansible-core milestone and python 3.10
integration-py3.11-2.14      -> Integration tests using ansible-core 2.14 and python 3.11
integration-py3.11-2.15      -> Integration tests using ansible-core 2.15 and python 3.11
integration-py3.11-devel     -> Integration tests using ansible-core devel and python 3.11
integration-py3.11-milestone -> Integration tests using ansible-core milestone and python 3.11
sanity-py3.8-2.9             -> Sanity tests using ansible 2.9 and python 3.8
sanity-py3.8-2.12            -> Sanity tests using ansible-core 2.12 and python 3.8
sanity-py3.8-2.13            -> Sanity tests using ansible-core 2.13 and python 3.8
sanity-py3.9-2.12            -> Sanity tests using ansible-core 2.12 and python 3.9
sanity-py3.9-2.13            -> Sanity tests using ansible-core 2.13 and python 3.9

each can be run independantly, and the virtual environment is available for each after the test run for additional troubleshooting or debugging.

This is all done with tools commonly used in the python ecosystem, tox and pytest.

@felixfontein
Copy link
Author

Combinations of Python version + core version make sense for integration tests. They can make sense for unit tests, for efficiency reasons, but they do not make sense for sanity tests. Most sanity tests have to be run once per core version, not multiple times. If you run them with every Python version supported by core, that's a waste of CI resources.

Considering how scarce CI resources are (ansible-community/community-topics#237), not wasting CI resources should be a high priority.

(Also I don't see why you need to do additional troubleshooting or debugging for sanity tests, except if the tests themselves are broken.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants