Skip to content
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

Community package requirements: sanity tests #108

Closed
gotmax23 opened this issue Jan 22, 2024 · 7 comments
Closed

Community package requirements: sanity tests #108

gotmax23 opened this issue Jan 22, 2024 · 7 comments

Comments

@gotmax23
Copy link

Community package requirements: sanity tests

(Note: This issue was filed in a semi-automated fashion. Let me know if you see errors in this issue.)

As per the Ansible community package inclusion requirements, collections must pass ansible-test sanity tests. Version 2.0.3 of cisco.intersight, corresponding to the 2.0.3 tag in this repo, fails one or more of the required sanity tests.

Please see the errors below and address them. If these issues aren't addressed within a reasonable time period, the collection may be subject to removal from Ansible.

Thank you for your efforts and for being part of the Ansible package! We appreciate it.


Sanity tests

The following tests were run using ansible-test version 2.16.1:

  • ansible-doc
  • compile
  • validate-modules
  • yamllint

Note that this is only a subset of the required sanity tests. Please make sure you run them in all in your CI.

Results

The test ansible-test sanity --test validate-modules [explain] failed with 15512 errors:

Issue Truncated. Click to read more..

@dsoper2
Copy link
Collaborator

dsoper2 commented Jan 26, 2024

Version 2.0.6 of the collection is passing sanity tests and has been certified in the Ansible Automation Hub. Logs of the ansible-test sanity and other required tests for certification are at https://console.redhat.com/ansible/automation-hub/repo/published/cisco/intersight/import-log/

@felixfontein
Copy link

The sanity tests do fail. When doing a clone of this repository and running ansible-test sanity --docker in it (ansible-core devel or ansible-core 2.17), there is a very long list of failures (all for the validate-modules test).

(Also what Automation Hub says on sanity tests does not count for the Ansible community package inclusion requirements.)

Similar to CiscoDevNet/ansible-ucs#40 (comment), there doesn't seem to be any CI in this repository, which is also a problem.

@dsoper2
Copy link
Collaborator

dsoper2 commented Aug 22, 2024

We'll work to setup CI for the repo - sorry that's not in place.
Can you share the errors you're seeing with ansible-test sanity? When run on the whole collection the output doesn't look right:
Running sanity test "validate-modules"
ERROR: Found 15582 validate-modules issue(s) which need to be resolved:
ERROR: plugins/modules/intersight_boot_order_policy.py:0:0: doc-choices-do-not-match-spec: Argument 'acs_control_gpu1state' in argument_spec defines choices as (['platform-default', 'enabled', 'disabled']) but documentation defines choices as ([])
...
The plugins/modules/intersight_boot_order_policy.py doesn't contain that code at all and ansible-test appears to actually be referring to code in plugins/modules/intersight_bios_policy.py. The arg spec and docs in plugins/modules/intersight_bios_policy.py is correct however and that file passes sanity tests:
ansible-test sanity plugins/modules/intersight_boot_order_policy.py
Running sanity test "action-plugin-docs"
...
Running sanity test "validate-modules"
Running sanity test "yamllint"

@felixfontein
Copy link

Hmm, that seems to be a bug in ansible-test then. The problem is that validate-modules imports all modules one by one and runs them, while mocking AnsibleModule.__init__. Since every single one of your modules uses the global intersight_argument_spec from module_utils and modifies it, you get these errors from the second module in the list on.

Applying the following patch gets rid of all these issues:

diff --git a/plugins/modules/intersight_bios_policy.py b/plugins/modules/intersight_bios_policy.py
index d2ec103..cfd1b93 100644
--- a/plugins/modules/intersight_bios_policy.py
+++ b/plugins/modules/intersight_bios_policy.py
@@ -4313,7 +4313,7 @@ def check_and_add_prop(prop, propKey, params, api_body):
 
 
 def main():
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         state={"type": "str", "choices": ['present', 'absent'], "default": "present"},
         organization={"type": "str", "default": "default"},
diff --git a/plugins/modules/intersight_boot_order_policy.py b/plugins/modules/intersight_boot_order_policy.py
index d66c015..c5d8177 100644
--- a/plugins/modules/intersight_boot_order_policy.py
+++ b/plugins/modules/intersight_boot_order_policy.py
@@ -319,7 +319,7 @@ def main():
             default='None',
         ),
     )
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         state=dict(type='str', choices=['present', 'absent'], default='present'),
         organization=dict(type='str', default='default'),
diff --git a/plugins/modules/intersight_imc_access_policy.py b/plugins/modules/intersight_imc_access_policy.py
index 27ebe80..5ca03a3 100644
--- a/plugins/modules/intersight_imc_access_policy.py
+++ b/plugins/modules/intersight_imc_access_policy.py
@@ -124,7 +124,7 @@ from ansible_collections.cisco.intersight.plugins.module_utils.intersight import
 
 
 def main():
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         state=dict(type='str', choices=['present', 'absent'], default='present'),
         organization=dict(type='str', default='default'),
diff --git a/plugins/modules/intersight_info.py b/plugins/modules/intersight_info.py
index b57aab8..6fab364 100644
--- a/plugins/modules/intersight_info.py
+++ b/plugins/modules/intersight_info.py
@@ -94,7 +94,7 @@ def get_servers(module, intersight):
 
 
 def main():
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         server_names=dict(type='list', elements='str'),
     )
diff --git a/plugins/modules/intersight_local_user_policy.py b/plugins/modules/intersight_local_user_policy.py
index 88b69b3..1459b79 100644
--- a/plugins/modules/intersight_local_user_policy.py
+++ b/plugins/modules/intersight_local_user_policy.py
@@ -176,7 +176,7 @@ def main():
         role=dict(type='str', choices=['admin', 'readonly', 'user'], required=True),
         password=dict(type='str', required=True, no_log=True),
     )
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         state=dict(type='str', choices=['present', 'absent'], default='present'),
         organization=dict(type='str', default='default'),
diff --git a/plugins/modules/intersight_ntp_policy.py b/plugins/modules/intersight_ntp_policy.py
index 01e7322..775d8e7 100644
--- a/plugins/modules/intersight_ntp_policy.py
+++ b/plugins/modules/intersight_ntp_policy.py
@@ -116,7 +116,7 @@ from ansible_collections.cisco.intersight.plugins.module_utils.intersight import
 
 
 def main():
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         state=dict(type='str', choices=['present', 'absent'], default='present'),
         organization=dict(type='str', default='default'),
diff --git a/plugins/modules/intersight_rest_api.py b/plugins/modules/intersight_rest_api.py
index 866c458..1b9e943 100644
--- a/plugins/modules/intersight_rest_api.py
+++ b/plugins/modules/intersight_rest_api.py
@@ -141,7 +141,7 @@ from ansible.module_utils.basic import AnsibleModule
 
 
 def main():
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         resource_path=dict(type='str', required=True),
         query_params=dict(type='dict'),
diff --git a/plugins/modules/intersight_server_profile.py b/plugins/modules/intersight_server_profile.py
index 8746bdc..b55ba1e 100644
--- a/plugins/modules/intersight_server_profile.py
+++ b/plugins/modules/intersight_server_profile.py
@@ -253,7 +253,7 @@ def post_profile_to_policy(intersight, moid, resource_path, policy_name):
 
 
 def main():
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         state=dict(type='str', choices=['present', 'absent'], default='present'),
         organization=dict(type='str', default='default'),
diff --git a/plugins/modules/intersight_target_claim.py b/plugins/modules/intersight_target_claim.py
index 97dcdc0..1d51fc7 100644
--- a/plugins/modules/intersight_target_claim.py
+++ b/plugins/modules/intersight_target_claim.py
@@ -106,7 +106,7 @@ from ansible.module_utils.basic import AnsibleModule
 
 
 def main():
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         claim_code=dict(type='str'),
         device_id=dict(type='str', required=True),
diff --git a/plugins/modules/intersight_virtual_media_policy.py b/plugins/modules/intersight_virtual_media_policy.py
index ba5b35d..abee9c5 100644
--- a/plugins/modules/intersight_virtual_media_policy.py
+++ b/plugins/modules/intersight_virtual_media_policy.py
@@ -268,7 +268,7 @@ def main():
         password=dict(type='str', no_log=True),
         authentication_protocol=dict(type='str', default='none'),
     )
-    argument_spec = intersight_argument_spec
+    argument_spec = intersight_argument_spec.copy()
     argument_spec.update(
         state=dict(type='str', choices=['present', 'absent'], default='present'),
         organization=dict(type='str', default='default'),

@felixfontein
Copy link

I filed a bug in ansible/ansible: ansible/ansible#83842

It still makes sense to apply the above patch, otherwise you won't be able to get CI pass until the ansible-test issue is fixed.

@dsoper2
Copy link
Collaborator

dsoper2 commented Aug 22, 2024

Thank you. I'll apply the patch and add CI to the repo.

@felixfontein
Copy link

The recommendation by the core team is also to copy the dictionary (ansible/ansible#83842 (comment)). I wouldn't be surprised if that problem won't get fixed, since a) a fix would be pretty complicated and/or expensive, and b) it's better for the modules to copy instead of modifying it anyway since that makes it easier to unit test complete modules. (If you would have unit tests for the modules where the whole module is started with some parameters, and the network APIs are mocked, you'd run into the same problem with unit tests, since they can also run in the same process and thus tests for different modules could cause problems with each other.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants