CiscoDevNet / ansible-aci

Cisco ACI Ansible Collection
https://galaxy.ansible.com/cisco/aci
GNU General Public License v3.0
143 stars 97 forks source link

[minor change] Add support for annotation in aci_rest module (#437) #497

Closed samiib closed 11 months ago

samiib commented 1 year ago

fixes #437

codecov[bot] commented 1 year ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (324327f) 35.32% compared to head (c6a7c6c) 96.25%.

Files Patch % Lines
plugins/modules/aci_rest.py 93.75% 0 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #497 +/- ## =========================================== + Coverage 35.32% 96.25% +60.92% =========================================== Files 230 231 +1 Lines 10610 10640 +30 Branches 1591 1601 +10 =========================================== + Hits 3748 10241 +6493 + Misses 6862 306 -6556 - Partials 0 93 +93 ``` | [Flag](https://app.codecov.io/gh/CiscoDevNet/ansible-aci/pull/497/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CiscoDevNet) | Coverage Δ | | |---|---|---| | [integration](https://app.codecov.io/gh/CiscoDevNet/ansible-aci/pull/497/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CiscoDevNet) | `94.71% <93.93%> (?)` | | | [sanity](https://app.codecov.io/gh/CiscoDevNet/ansible-aci/pull/497/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CiscoDevNet) | `35.27% <18.18%> (-0.06%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CiscoDevNet#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

samiib commented 12 months ago

cm_add_tenant.proposed.fvTenant.attributes.annotation == "orchestrator:ansible" check is failing.

@sajagana This check in the xml_string tests should be fixed now.

samiib commented 12 months ago

Also update the tests/integration/targets/aci_rest/tasks/error_handling.yml task: - name: Verify error_on_input_validation

-    - "error_on_input_validation.msg == 'APIC Error 801: property descr of tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
+    - "error_on_input_validation.msg == 'APIC Error 801: property descr of uni/tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"

@sajagana, Not sure about this one because the test was passing when I ran it myself. Just wondering which APIC did you use to test on?

akinross commented 11 months ago

Also update the tests/integration/targets/aci_rest/tasks/error_handling.yml task: - name: Verify error_on_input_validation

-    - "error_on_input_validation.msg == 'APIC Error 801: property descr of tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
+    - "error_on_input_validation.msg == 'APIC Error 801: property descr of uni/tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"

@sajagana, Not sure about this one because the test was passing when I ran it myself. Just wondering which APIC did you use to test on?

I'm seeing the same error now when running tests ( did not see runs before ). On november 28th the APICs where upgraded as I recall, perhaps the return message has changed?

samiib commented 11 months ago

Also update the tests/integration/targets/aci_rest/tasks/error_handling.yml task: - name: Verify error_on_input_validation

-    - "error_on_input_validation.msg == 'APIC Error 801: property descr of tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"
+    - "error_on_input_validation.msg == 'APIC Error 801: property descr of uni/tn-ansible_test failed validation for value \\'This is an [invalid] description\\''"

@sajagana, Not sure about this one because the test was passing when I ran it myself. Just wondering which APIC did you use to test on?

I'm seeing the same error now when running tests ( did not see runs before ). On november 28th the APICs where upgraded as I recall, perhaps the return message has changed?

@sajagana & @akinross

I have updated my branch with the latest changes from master and have been able to reproduce the failed assertion. It appears the error message has changed between different versions of APIC. Some versions include uni/ in the error and other versions don't. To account for this difference I changed the assertion to use a simple regex so the test will pass for this message variation.