Skip to content

[minor change] New module for out-of-band contract creation (DCNE-219) #700

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

Ziaf007
Copy link

@Ziaf007 Ziaf007 commented Oct 24, 2024

added in module for oob_contract creation.
Updated test files for this as well.

@akinross akinross added the jira-sync Sync this issue to Jira label Oct 24, 2024
@github-actions github-actions bot changed the title [minor change] New module for out-of-band contract creation [minor change] New module for out-of-band contract creation (DCNE-219) Oct 24, 2024
@akinross akinross self-requested a review December 16, 2024 13:52
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 21.25%. Comparing base (2fdee82) to head (9cddf5c).
Report is 7 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (2fdee82) and HEAD (9cddf5c). Click for more details.

HEAD has 13 uploads less than BASE
Flag BASE (2fdee82) HEAD (9cddf5c)
unit 8 1
integration 2 0
sanity 4 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #700       +/-   ##
===========================================
- Coverage   95.68%   21.25%   -74.44%     
===========================================
  Files         270        4      -266     
  Lines       12404     1087    -11317     
  Branches     1869      246     -1623     
===========================================
- Hits        11869      231    -11638     
- Misses        404      836      +432     
+ Partials      131       20      -111     
Flag Coverage Δ
integration ?
sanity ?
unit 21.25% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

2) Updated more assertations in test.py
3) merged the root and subclass1 into one single root class
tests succeed finely
PLAY RECAP *********************************************************************
azure_cloud                : ok=8    changed=0    unreachable=0    failed=0    skipped=22   rescued=0    ignored=0
cn-dmz-apic-m1-02-v42      : ok=7    changed=0    unreachable=0    failed=0    skipped=23   rescued=0    ignored=0
cn-dmz-apic-m1-03-v52      : ok=26   changed=7    unreachable=0    failed=0    skipped=4    rescued=0    ignored=2
cn-dmz-apic-m1-04-v602h    : ok=26   changed=7    unreachable=0    failed=0    skipped=4    rescued=0    ignored=2
@akinross akinross self-requested a review February 3, 2025 08:50
2) added an example with Priority attribute set

.
shrsr
shrsr previously approved these changes Mar 20, 2025
Copy link
Collaborator

@shrsr shrsr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shrsr shrsr requested a review from akinross March 20, 2025 17:58
@shrsr shrsr requested a review from anvitha-jain March 23, 2025 00:48
akinross
akinross previously approved these changes Mar 25, 2025
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

gmicol
gmicol previously approved these changes Mar 25, 2025
Copy link
Collaborator

@gmicol gmicol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Ziaf007 Ziaf007 dismissed stale reviews from gmicol, akinross, and shrsr via 0144eea March 25, 2025 22:00
@shrsr shrsr requested review from shrsr, gmicol and akinross March 25, 2025 22:27
shrsr
shrsr previously approved these changes Mar 25, 2025
Copy link
Collaborator

@shrsr shrsr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sanity failures still there please fix

ERROR: plugins/modules/aci_oob_contract.py:0:0: parameter-type-not-in-doc: Argument 'name_alias' in argument_spec defines type as 'str' but documentation doesn't define type
ERROR: plugins/modules/aci_oob_contract.py:0:0: undocumented-parameter: Argument 'name_alias' is listed in the argument_spec, but not documented in the module documentation

- present_check_mode is changed
- present_check_mode.previous == []
- present_check_mode.previous == present_check_mode.current
- present_check_mode.sent.vzOOBBrCP.attributes.name == 'anstest'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- present_check_mode.sent.vzOOBBrCP.attributes.name == 'anstest'
- present_check_mode.proposed.vzOOBBrCP.attributes.name == 'anstest'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

- name: Create OOB Contract
cisco.aci.aci_oob_contract: &aci_oob_contract_present
<<: *aci_oob_contract_present_CM
state: "{{ fakevar | default(omit) }}" # >>>> DEFAULTS TO 'PRESENT'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just set present?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for testing the default value when omitted

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok then add that that is the intention of your test

- absent_check_mode is changed
- absent_check_mode.previous != []
- contract_absent is changed
- contract_absent.previous == absent_check_mode.previous
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you assert previous attributes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira-sync Sync this issue to Jira
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants