unit test helper: big revamp #8894

russoz · 2024-09-22T05:29:59Z

SUMMARY

Revamped the test helper to be able to support more than just run_command(). This PR makes the code more generic and expansible, allowing for easier tests in the future.

ISSUE TYPE

Test Pull Request

COMPONENT NAME

tests/unit/plugins/modules/conftest.py
tests/unit/plugins/modules/helper.py
tests/unit/plugins/modules/test_cpanm.py
tests/unit/plugins/modules/test_cpanm.yaml
tests/unit/plugins/modules/test_django_check.py
tests/unit/plugins/modules/test_django_check.yaml
tests/unit/plugins/modules/test_django_check.yaml.license
tests/unit/plugins/modules/test_django_command.py
tests/unit/plugins/modules/test_django_command.yaml
tests/unit/plugins/modules/test_django_createcachetable.py
tests/unit/plugins/modules/test_django_createcachetable.yaml
tests/unit/plugins/modules/test_facter_facts.py
tests/unit/plugins/modules/test_facter_facts.yaml
tests/unit/plugins/modules/test_gconftool2.py
tests/unit/plugins/modules/test_gconftool2.yaml
tests/unit/plugins/modules/test_gconftool2_info.py
tests/unit/plugins/modules/test_gconftool2_info.yaml
tests/unit/plugins/modules/test_gio_mime.py
tests/unit/plugins/modules/test_gio_mime.yaml
tests/unit/plugins/modules/test_opkg.py
tests/unit/plugins/modules/test_opkg.yaml
tests/unit/plugins/modules/test_puppet.py
tests/unit/plugins/modules/test_puppet.yaml
tests/unit/plugins/modules/test_snap.py
tests/unit/plugins/modules/test_xfconf.py
tests/unit/plugins/modules/test_xfconf.yaml
tests/unit/plugins/modules/test_xfconf_info.py
tests/unit/plugins/modules/test_xfconf_info.yaml

russoz · 2024-09-22T05:53:15Z

The CI error seems to be unrelated with the change. @felixfontein could you please confirm? TIA

felixfontein

It seems that somewhere the order of the tests comes from a dictionary. In Python 3.5 (where the tests fail) the order is 'random', while in Python 3.6+ it is insertion order. (No idea why it works in 2.7 since there it should be 'random' as well...)

tests/unit/plugins/modules/helper.py

russoz · 2024-09-22T22:05:35Z

It seems that somewhere the order of the tests comes from a dictionary. In Python 3.5 (where the tests fail) the order is 'random', while in Python 3.6+ it is insertion order. (No idea why it works in 2.7 since there it should be 'random' as well...)

I do recall something about that in 2.7 it was "random" but the seed to the random generator was kinda fixed, so the order of the keys was deterministic across multiple runs of the script. They improved that with 3.5, and I had not kept track, I did not know 3.6 onwards was using insertion order. Anyways, it is not something to rely on.

russoz · 2024-09-27T22:30:43Z

hi @felixfontein anything else I need to adjust in this one? TIA

- TestCaseContext fixture no longer need to autouse=True - Helper.from_module() allows extra param to specify yaml file - test_django_check: adjusted .py and .yaml

- now it works not only in parametrized functions but also directly with args

- improved encapsulation, class Helper no longer knows details about test cases - test functions no longer parametrized, that allows using test case fixtures per test function - renamed 'context' to 'mock'

Co-authored-by: Felix Fontein <felix@fontein.de>

russoz · 2024-09-28T04:35:43Z

It seems that somewhere the order of the tests comes from a dictionary. In Python 3.5 (where the tests fail) the order is 'random', while in Python 3.6+ it is insertion order. (No idea why it works in 2.7 since there it should be 'random' as well...)

Yup, I gave it another look. I think I know what it is: the order of the tests, in the code, is a list, but as the test functions are dynamically added to the module, they are included into the underlying dict that supports the module's attributes. Not sure it does explain it.

I chose not parameterizing the test function so that fixtures could be used (with pytest.mark.usefixtures) per test function, rather than for all test cases.

Anyways, whatever test code is performing this comparison between collected tests is clearly relying on the order provided by the module. I guess for the time being the only option I have is to create one single function and parameterize it. This will become a problem if developers use the test helper and create extra tests within the same test_module. Anyway I can skip Python 3.5?

russoz · 2024-09-28T04:46:36Z

the problem lies in the pytest-xdist plugin used here:

https://pytest-xdist.readthedocs.io/en/stable/known-limitations.html#order-and-amount-of-test-must-be-consistent

I don't want to chase the rabbit down that hole, so I will revert to a parameterized test function.

felixfontein · 2024-09-28T05:17:04Z

hi @felixfontein anything else I need to adjust in this one? TIA

The tests were failing on Python 3.5, as you probably noticed :)

community.general 11.0.0 will drop support for Python 3.5 (though not for 2.7), and 12.0.0 will drop support for Python 2.7 as well, so at that point you can do this again :)

patchback · 2024-09-28T05:17:40Z

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/8ef77d8664598154fdd51bf522c0afc62fe36b65/pr-8894

Backported as #8943

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

felixfontein · 2024-09-28T05:17:50Z

@russoz thanks for your contribution!

* initial commit * multiple changes: - TestCaseContext fixture no longer need to autouse=True - Helper.from_module() allows extra param to specify yaml file - test_django_check: adjusted .py and .yaml * set fixtures per testcase * set fixtures per testcase * rollback to original state * patch_ansible_module fixture - now it works not only in parametrized functions but also directly with args * tests/unit/plugins/modules/helper.py - improved encapsulation, class Helper no longer knows details about test cases - test functions no longer parametrized, that allows using test case fixtures per test function - renamed 'context' to 'mock' * enable Helper.from_list(), better param name 'ansible_module' * adjusted test fiels to new helper * remove unnecessary .license file * fix bracket * fix reference name * Update tests/unit/plugins/modules/helper.py Co-authored-by: Felix Fontein <felix@fontein.de> * revert to parametrized test func instead of multiple funcs --------- Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit 8ef77d8)

…8943) unit test helper: big revamp (#8894) * initial commit * multiple changes: - TestCaseContext fixture no longer need to autouse=True - Helper.from_module() allows extra param to specify yaml file - test_django_check: adjusted .py and .yaml * set fixtures per testcase * set fixtures per testcase * rollback to original state * patch_ansible_module fixture - now it works not only in parametrized functions but also directly with args * tests/unit/plugins/modules/helper.py - improved encapsulation, class Helper no longer knows details about test cases - test functions no longer parametrized, that allows using test case fixtures per test function - renamed 'context' to 'mock' * enable Helper.from_list(), better param name 'ansible_module' * adjusted test fiels to new helper * remove unnecessary .license file * fix bracket * fix reference name * Update tests/unit/plugins/modules/helper.py Co-authored-by: Felix Fontein <felix@fontein.de> * revert to parametrized test func instead of multiple funcs --------- Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit 8ef77d8) Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

* initial commit * multiple changes: - TestCaseContext fixture no longer need to autouse=True - Helper.from_module() allows extra param to specify yaml file - test_django_check: adjusted .py and .yaml * set fixtures per testcase * set fixtures per testcase * rollback to original state * patch_ansible_module fixture - now it works not only in parametrized functions but also directly with args * tests/unit/plugins/modules/helper.py - improved encapsulation, class Helper no longer knows details about test cases - test functions no longer parametrized, that allows using test case fixtures per test function - renamed 'context' to 'mock' * enable Helper.from_list(), better param name 'ansible_module' * adjusted test fiels to new helper * remove unnecessary .license file * fix bracket * fix reference name * Update tests/unit/plugins/modules/helper.py Co-authored-by: Felix Fontein <felix@fontein.de> * revert to parametrized test func instead of multiple funcs --------- Co-authored-by: Felix Fontein <felix@fontein.de>

ansibullbot added new_plugin New plugin tests tests unit tests/unit labels Sep 22, 2024

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Sep 22, 2024

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-9 Automatically create a backport for the stable-9 branch labels Sep 22, 2024

felixfontein approved these changes Sep 22, 2024

View reviewed changes

felixfontein reviewed Sep 22, 2024

View reviewed changes

tests/unit/plugins/modules/helper.py Outdated Show resolved Hide resolved

russoz and others added 13 commits September 28, 2024 14:25

initial commit

8f91158

multiple changes:

7ee367e

- TestCaseContext fixture no longer need to autouse=True - Helper.from_module() allows extra param to specify yaml file - test_django_check: adjusted .py and .yaml

set fixtures per testcase

38b288e

set fixtures per testcase

25f5d63

rollback to original state

1b8503a

patch_ansible_module fixture

f6d7eaa

- now it works not only in parametrized functions but also directly with args

tests/unit/plugins/modules/helper.py

4541983

- improved encapsulation, class Helper no longer knows details about test cases - test functions no longer parametrized, that allows using test case fixtures per test function - renamed 'context' to 'mock'

enable Helper.from_list(), better param name 'ansible_module'

45ed7ba

adjusted test fiels to new helper

6622e70

remove unnecessary .license file

eae0de9

fix bracket

dc8b576

fix reference name

1064a59

Update tests/unit/plugins/modules/helper.py

52e668c

Co-authored-by: Felix Fontein <felix@fontein.de>

russoz force-pushed the generic_test_helper branch from 5cd6b5b to 52e668c Compare September 28, 2024 02:26

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Sep 28, 2024

revert to parametrized test func instead of multiple funcs

b18214f

felixfontein approved these changes Sep 28, 2024

View reviewed changes

felixfontein merged commit 8ef77d8 into ansible-collections:main Sep 28, 2024
141 checks passed

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 28, 2024

patchback bot mentioned this pull request Sep 28, 2024

[PR #8894/8ef77d86 backport][stable-9] unit test helper: big revamp #8943

Merged

russoz deleted the generic_test_helper branch September 28, 2024 05:53

russoz mentioned this pull request Oct 2, 2024

krb_ticket: Create module #8953

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

unit test helper: big revamp #8894

unit test helper: big revamp #8894

russoz commented Sep 22, 2024

russoz commented Sep 22, 2024

felixfontein left a comment

russoz commented Sep 22, 2024 •

edited

Loading

russoz commented Sep 27, 2024

russoz commented Sep 28, 2024

russoz commented Sep 28, 2024 •

edited

Loading

felixfontein commented Sep 28, 2024

patchback bot commented Sep 28, 2024 •

edited

Loading

felixfontein commented Sep 28, 2024

unit test helper: big revamp #8894

unit test helper: big revamp #8894

Conversation

russoz commented Sep 22, 2024

SUMMARY

ISSUE TYPE

COMPONENT NAME

russoz commented Sep 22, 2024

felixfontein left a comment

Choose a reason for hiding this comment

russoz commented Sep 22, 2024 • edited Loading

russoz commented Sep 27, 2024

russoz commented Sep 28, 2024

russoz commented Sep 28, 2024 • edited Loading

felixfontein commented Sep 28, 2024

patchback bot commented Sep 28, 2024 • edited Loading

Backport to stable-9: 💚 backport PR created

felixfontein commented Sep 28, 2024

russoz commented Sep 22, 2024 •

edited

Loading

russoz commented Sep 28, 2024 •

edited

Loading

patchback bot commented Sep 28, 2024 •

edited

Loading