Fb keycloak client improvement #9644

amPrimeSign · 2025-01-28T08:37:57Z

SUMMARY

Fix and improve existing tests for keycloak_client module

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

keycloak_client

ADDITIONAL INFORMATION

Run ansible-test integration -v keycloak_client --allow-unsupported --docker fedora40 --docker-network host

Before:

[...]
TASK [keycloak_client : Assert changes not detected in last two tasks (desire when same, and check)] ***
    task path: /root/ansible_collections/community/general/tests/output/.tmp/integration/keycloak_client-p3ttqf7d-ÅÑŚÌβŁÈ/tests/integration/targets/keycloak_client/tasks/main.yml:79
    fatal: [testhost]: FAILED! => {
        "assertion": "check_client_when_present_and_same is not changed",
        "changed": false,
        "evaluated_to": false,
        "msg": "Assertion failed"
    }
[...]

After:

[...]
TASK [keycloak_client : Assert changes not detected in last two tasks (desire when same, and check)] ***
task path: /root/ansible_collections/community/general/tests/output/.tmp/integration/keycloak_client-bhrae54u-ÅÑŚÌβŁÈ/tests/integration/targets/keycloak_client/tasks/main.yml:79
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}
[...]

TASK [keycloak_client : Assert changes not detected in last two tasks (desire when same, and check)] *** task path: /root/ansible_collections/community/general/tests/output/.tmp/integration/keycloak_client-p3ttqf7d-ÅÑŚÌβŁÈ/tests/integration/targets/keycloak_client/tasks/main.yml:79 fatal: [testhost]: FAILED! => { "assertion": "check_client_when_present_and_same is not changed", "changed": false, "evaluated_to": false, "msg": "Assertion failed" }

…e in examples

ansibullbot · 2025-01-28T08:51:11Z

cc @eikef @mattock @ndclt @thomasbach-dev
click here for bot help

russoz

hi @amPrimeSign thanks for your contribution

I have a couple of suggestions on it, and I think the test file actually needs fixing.

tests/integration/targets/keycloak_client/vars/main.yml

plugins/modules/keycloak_client.py

Co-authored-by: Alexei Znamensky <[email protected]>

russoz

hi @amPrimeSign thanks for the adjustments.

The changelog might use a bit of a tweak, and I left another comment with a suggestion that you may disregard at will.

changelogs/fragments/9644-kc_client-test-improvement-and-fix.yaml

russoz · 2025-01-29T10:22:49Z

plugins/modules/keycloak_client.py

+            if 'config' in mapper:
+                for key, value in mapper['config'].items():
+                    if isinstance(value, bool):
+                        mapper['config'][key] = str(value).lower()


I might be overthinking this, but you are modifying items of a dict that you are iterating over. I remember this causing problems in some versions of Python back in the day. But assuming that the tests cover this part of the code, it should be alright.

Also, an alternative way that would not incur in the non-existent problem 😆 (feel free to disregard) to implement this could be:

Suggested change

if 'config' in mapper:

for key, value in mapper['config'].items():

if isinstance(value, bool):

mapper['config'][key] = str(value).lower()

if 'config' in mapper:

bool_items = {k: str(v).lower() for k,v in mapper['config'].items() if isinstance(v, bool)}

mapper['config'].update(bool_items)

I think editing an existing value should be ok, but I'm not a Python expert. For me, both options are fine, please pick your preferred one.

Modifying dict values is fine, deleting or adding items would cause issues.

Thanks for chipping in @apollo13 !
@amPrimeSign so there you go, we're good the way it is now. :-)

@russoz Thanks :)

Co-authored-by: Alexei Znamensky <[email protected]>

russoz

LGTM

patchback · 2025-02-01T18:16:57Z

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/250dc1139c53ab429196fe64d8f3676e75977c61/pr-9644

Backported as #9670

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

* Fix for failed test TASK [keycloak_client : Assert changes not detected in last two tasks (desire when same, and check)] *** task path: /root/ansible_collections/community/general/tests/output/.tmp/integration/keycloak_client-p3ttqf7d-ÅÑŚÌβŁÈ/tests/integration/targets/keycloak_client/tasks/main.yml:79 fatal: [testhost]: FAILED! => { "assertion": "check_client_when_present_and_same is not changed", "changed": false, "evaluated_to": false, "msg": "Assertion failed" } * Improved test data to test more scenarios, e.g documentation uses True in examples * Normalize values in config * add changelog * Apply suggestions from code review Co-authored-by: Alexei Znamensky <[email protected]> * Update tests/integration/targets/keycloak_client/vars/main.yml Co-authored-by: Alexei Znamensky <[email protected]> * Update changelogs/fragments/9644-kc_client-test-improvement-and-fix.yaml Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 250dc11)

patchback · 2025-02-01T18:17:06Z

Backport to stable-10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-10/250dc1139c53ab429196fe64d8f3676e75977c61/pr-9644

Backported as #9671

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

felixfontein · 2025-02-01T18:17:15Z

@amPrimeSign thanks for your contribution!
@russoz @apollo13 thanks for reviewing!

* Fix for failed test TASK [keycloak_client : Assert changes not detected in last two tasks (desire when same, and check)] *** task path: /root/ansible_collections/community/general/tests/output/.tmp/integration/keycloak_client-p3ttqf7d-ÅÑŚÌβŁÈ/tests/integration/targets/keycloak_client/tasks/main.yml:79 fatal: [testhost]: FAILED! => { "assertion": "check_client_when_present_and_same is not changed", "changed": false, "evaluated_to": false, "msg": "Assertion failed" } * Improved test data to test more scenarios, e.g documentation uses True in examples * Normalize values in config * add changelog * Apply suggestions from code review Co-authored-by: Alexei Znamensky <[email protected]> * Update tests/integration/targets/keycloak_client/vars/main.yml Co-authored-by: Alexei Znamensky <[email protected]> * Update changelogs/fragments/9644-kc_client-test-improvement-and-fix.yaml Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 250dc11)

…#9670) Fb keycloak client improvement (#9644) * Fix for failed test TASK [keycloak_client : Assert changes not detected in last two tasks (desire when same, and check)] *** task path: /root/ansible_collections/community/general/tests/output/.tmp/integration/keycloak_client-p3ttqf7d-ÅÑŚÌβŁÈ/tests/integration/targets/keycloak_client/tasks/main.yml:79 fatal: [testhost]: FAILED! => { "assertion": "check_client_when_present_and_same is not changed", "changed": false, "evaluated_to": false, "msg": "Assertion failed" } * Improved test data to test more scenarios, e.g documentation uses True in examples * Normalize values in config * add changelog * Apply suggestions from code review Co-authored-by: Alexei Znamensky <[email protected]> * Update tests/integration/targets/keycloak_client/vars/main.yml Co-authored-by: Alexei Znamensky <[email protected]> * Update changelogs/fragments/9644-kc_client-test-improvement-and-fix.yaml Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 250dc11) Co-authored-by: amPrimeSign <[email protected]>

#9671) Fb keycloak client improvement (#9644) * Fix for failed test TASK [keycloak_client : Assert changes not detected in last two tasks (desire when same, and check)] *** task path: /root/ansible_collections/community/general/tests/output/.tmp/integration/keycloak_client-p3ttqf7d-ÅÑŚÌβŁÈ/tests/integration/targets/keycloak_client/tasks/main.yml:79 fatal: [testhost]: FAILED! => { "assertion": "check_client_when_present_and_same is not changed", "changed": false, "evaluated_to": false, "msg": "Assertion failed" } * Improved test data to test more scenarios, e.g documentation uses True in examples * Normalize values in config * add changelog * Apply suggestions from code review Co-authored-by: Alexei Znamensky <[email protected]> * Update tests/integration/targets/keycloak_client/vars/main.yml Co-authored-by: Alexei Znamensky <[email protected]> * Update changelogs/fragments/9644-kc_client-test-improvement-and-fix.yaml Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]> (cherry picked from commit 250dc11) Co-authored-by: amPrimeSign <[email protected]>

amPrimeSign · 2025-02-03T09:27:00Z

@russoz, @felixfontein thanks for the feedback and for the quick processing

* Fix for failed test TASK [keycloak_client : Assert changes not detected in last two tasks (desire when same, and check)] *** task path: /root/ansible_collections/community/general/tests/output/.tmp/integration/keycloak_client-p3ttqf7d-ÅÑŚÌβŁÈ/tests/integration/targets/keycloak_client/tasks/main.yml:79 fatal: [testhost]: FAILED! => { "assertion": "check_client_when_present_and_same is not changed", "changed": false, "evaluated_to": false, "msg": "Assertion failed" } * Improved test data to test more scenarios, e.g documentation uses True in examples * Normalize values in config * add changelog * Apply suggestions from code review Co-authored-by: Alexei Znamensky <[email protected]> * Update tests/integration/targets/keycloak_client/vars/main.yml Co-authored-by: Alexei Znamensky <[email protected]> * Update changelogs/fragments/9644-kc_client-test-improvement-and-fix.yaml Co-authored-by: Alexei Znamensky <[email protected]> --------- Co-authored-by: Alexei Znamensky <[email protected]>

amPrimeSign added 3 commits January 28, 2025 08:29

Improved test data to test more scenarios, e.g documentation uses Tru…

3a549c1

…e in examples

Normalize values in config

0a127c9

amPrimeSign force-pushed the fb-keycloak-client-improvement branch 2 times, most recently from 431a6c5 to 0a088f8 Compare January 28, 2025 08:49

add changelog

8495695

amPrimeSign force-pushed the fb-keycloak-client-improvement branch from 0a088f8 to 8495695 Compare January 28, 2025 08:50

russoz reviewed Jan 28, 2025

View reviewed changes

tests/integration/targets/keycloak_client/vars/main.yml Outdated Show resolved Hide resolved

plugins/modules/keycloak_client.py Outdated Show resolved Hide resolved

plugins/modules/keycloak_client.py Outdated Show resolved Hide resolved

Apply suggestions from code review

81895f1

Co-authored-by: Alexei Znamensky <[email protected]>

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

Update tests/integration/targets/keycloak_client/vars/main.yml

dd8316f

Co-authored-by: Alexei Znamensky <[email protected]>

russoz reviewed Jan 29, 2025

View reviewed changes

Update changelogs/fragments/9644-kc_client-test-improvement-and-fix.yaml

c4873f5

Co-authored-by: Alexei Znamensky <[email protected]>

russoz approved these changes Jan 30, 2025

View reviewed changes

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Feb 1, 2025

felixfontein merged commit 250dc11 into ansible-collections:main Feb 1, 2025
138 checks passed

patchback bot mentioned this pull request Feb 1, 2025

[PR #9644/250dc113 backport][stable-9] Fb keycloak client improvement #9670

Merged

patchback bot mentioned this pull request Feb 1, 2025

[PR #9644/250dc113 backport][stable-10] Fb keycloak client improvement #9671

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fb keycloak client improvement #9644

Fb keycloak client improvement #9644

amPrimeSign commented Jan 28, 2025 •

edited

Loading

ansibullbot commented Jan 28, 2025

russoz left a comment

russoz left a comment

russoz Jan 29, 2025

amPrimeSign Jan 30, 2025 •

edited

Loading

apollo13 Jan 30, 2025

russoz Jan 30, 2025

amPrimeSign Jan 31, 2025

russoz left a comment

patchback bot commented Feb 1, 2025 •

edited

Loading

patchback bot commented Feb 1, 2025 •

edited

Loading

felixfontein commented Feb 1, 2025

amPrimeSign commented Feb 3, 2025

Fb keycloak client improvement #9644

Fb keycloak client improvement #9644

Conversation

amPrimeSign commented Jan 28, 2025 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibullbot commented Jan 28, 2025

russoz left a comment

Choose a reason for hiding this comment

russoz left a comment

Choose a reason for hiding this comment

russoz Jan 29, 2025

Choose a reason for hiding this comment

amPrimeSign Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

apollo13 Jan 30, 2025

Choose a reason for hiding this comment

russoz Jan 30, 2025

Choose a reason for hiding this comment

amPrimeSign Jan 31, 2025

Choose a reason for hiding this comment

russoz left a comment

Choose a reason for hiding this comment

patchback bot commented Feb 1, 2025 • edited Loading

Backport to stable-9: 💚 backport PR created

patchback bot commented Feb 1, 2025 • edited Loading

Backport to stable-10: 💚 backport PR created

felixfontein commented Feb 1, 2025

amPrimeSign commented Feb 3, 2025

amPrimeSign commented Jan 28, 2025 •

edited

Loading

amPrimeSign Jan 30, 2025 •

edited

Loading

patchback bot commented Feb 1, 2025 •

edited

Loading

patchback bot commented Feb 1, 2025 •

edited

Loading