More fixes to zabbix_action.py

[BGmot]

SUMMARY

Fixes #661
Fixes action module behaviour: always showing changed=True even if nothing has changed

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

zabbix_action.py

[BGmot]

I don't understand why tests expect changed to be true when creating the same action again.

[D3DeFi]

I don't understand why tests expect changed to be true when creating the same action again.

Thank you for taking initiative in solving this @BGmot !

The problem with tests is that they usually run the first time and if they fail, ansible re-runs them with ASNIBLE_DEBUG=1 and -vvv. This leads to a confusing result, because failed tests before didn't clean up after them.

Your problem happens much sooner in the failed test log (around line ~70 in the first one (4.0@stable-2.10_py.27):

TASK [test_zabbix_action : test - create new action (again)] *******************
changed: [testhost] => {"changed": true, "msg": "Action Updated: ExampleTriggerAction, ID: {u'actionids': [7]}"}

TASK [test_zabbix_action : assert] *********************************************
fatal: [testhost]: FAILED! => {
    "assertion": "zbxaction_new.changed is sameas False", 
    "changed": false, 
    "evaluated_to": false, 
    "msg": "Assertion failed"
}

PLAY RECAP *********************************************************************
testhost                   : ok=9    changed=4    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   

So what happens is that test wants to re-run same task again and expect nothing to be changed. Based on your PR it is possible that we expect wrong behavior to be correct in the tests and they should be changed.

tl;dr something in the patch breaks idempotency of the module (for at least older zabbix versions)

[BGmot]

This is exactly what I am saying. After fixing the issue with "value" attribute and running real-life tests with creating, updating and creating an Action with exactly the same parameters as existing one I noticed that in the latter case it always shows 'changed=ture' instead of expected 'changed=false' as no parameters changed. After some digging I found during current and new objects comparison for some reason we discard "recovery_operations" and "acknowledge_operations" so I fixed that in
9440c4f and got expected behavior. But now tests fails. I am not extremely familiar with how tests work (in fact this is the first time I am coding for Ansible) so I'll take a look why they fail and try to understand and either fix the code or the tests.

[codecov]

Codecov Report

Merging #667 (42db057) into main (deba504) will decrease coverage by 0.33%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #667      +/-   ##
==========================================
- Coverage   70.98%   70.65%   -0.34%     
==========================================
  Files          27       27              
  Lines        3550     3568      +18     
  Branches      920      929       +9     
==========================================
+ Hits         2520     2521       +1     
- Misses        751      765      +14     
- Partials      279      282       +3     

Impacted Files	Coverage Δ
.../community/zabbix/plugins/modules/zabbix_action.py	64.24% <0.00%> (-2.16%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update deba504...42db057. Read the comment docs.

[BGmot]

Ok, I've fixed changed=false/true for older versions of Zabbix. Apparently action.get in Zabbix 5.0 returns recoveryOperations and acknowledgeOperations properties while in Zabbix 6.0 it returns recovery_operations and acknowledge_operations.
Now codecov/project test is failing. No idea what it means -( but I have a feeling that adding Zabbix 6.0 to the tests matrix will help with this test (see PS).

PS: in our tests matrix we don't have Zabbix 6.0, should we add one?

[D3DeFi]

Ok, I've fixed changed=false/true for older versions of Zabbix. Apparently action.get in Zabbix 5.0 returns recoveryOperations and acknowledgeOperations properties while in Zabbix 6.0 it returns recovery_operations and acknowledge_operations. Now codecov/project test is failing. No idea what it means -( but I have a feeling that adding Zabbix 6.0 to the tests matrix will help with this test (see PS).

PS: in our tests matrix we don't have Zabbix 6.0, should we add one?

Dont worry about codecov, those are tests that fail if coverage of our tests is lower than before PR contribution. I seem to never satisfy that check either :)

[D3DeFi]

This now looks good to me. Thank you for taking time to contribute this patch. Very nice job! 🚀