Move ping and win_ping closer together (#26028)

So in an effort to verify if Windows modules are feature complete compared to the python equivalent, I stumbled upon these differences. This PR includes: - Add missing 'data' option from documentation - Simplify ping module - Update integration tests to test exception
ansible · Jun 28, 2017 · 5be32aa · 5be32aa
1 parent 56d33a2
commit 5be32aa
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 56 deletions.
diff --git a/lib/ansible/modules/system/ping.py b/lib/ansible/modules/system/ping.py
@@ -24,12 +24,11 @@
                     'status': ['stableinterface'],
                     'supported_by': 'core'}
 
-
 DOCUMENTATION = '''
 ---
 module: ping
 version_added: historical
-short_description: Try to connect to host, verify a usable python and return C(pong) on success.
+short_description: Try to connect to host, verify a usable python and return C(pong) on success
 description:
    - A trivial test module, this module always returns C(pong) on successful
      contact. It does not make sense in playbooks, but it is useful from
@@ -38,15 +37,35 @@
    - For Windows targets, use the M(ping) module instead.
 notes:
    - For Windows targets, use the M(ping) module instead.
-options: {}
+options:
+  data:
+    description:
+      - Data to return for the C(ping) return value.
+      - If this parameter is set to C(crash), the module will cause an exception.
+    default: pong
 author:
-    - "Ansible Core Team"
-    - "Michael DeHaan"
+    - Ansible Core Team
+    - Michael DeHaan
 '''
 
 EXAMPLES = '''
 # Test we can logon to 'webservers' and execute python with json lib.
-ansible webservers -m ping
+# ansible webservers -m ping
+
+# Example from an Ansible Playbook
+- ping:
+
+# Induce an exception to see what happens
+- ping:
+    data: crash
+'''
+
+RETURN = '''
+ping:
+    description: value provided with the data parameter
+    returned: success
+    type: string
+    sample: pong
 '''
 
 from ansible.module_utils.basic import AnsibleModule
@@ -55,15 +74,18 @@
 def main():
     module = AnsibleModule(
         argument_spec=dict(
-            data=dict(required=False, default=None),
+            data=dict(type='str', default='pong'),
         ),
         supports_check_mode=True
     )
-    result = dict(ping='pong')
-    if module.params['data']:
-        if module.params['data'] == 'crash':
-            raise Exception("boom")
-        result['ping'] = module.params['data']
+
+    if module.params['data'] == 'crash':
+        raise Exception("boom")
+
+    result = dict(
+        ping=module.params['data'],
+    )
+
     module.exit_json(**result)
 
 

diff --git a/lib/ansible/modules/windows/win_ping.py b/lib/ansible/modules/windows/win_ping.py
@@ -25,7 +25,6 @@
                     'status': ['stableinterface'],
                     'supported_by': 'core'}
 
-
 DOCUMENTATION = r'''
 ---
 module: win_ping
@@ -38,11 +37,13 @@
 options:
   data:
     description:
-      - Alternate data to return instead of 'pong'
-    default: 'pong'
+      - Alternate data to return instead of 'pong'.
+      - If this parameter is set to C(crash), the module will cause an exception.
+    default: pong
 notes:
   - For non-Windows targets, use the M(ping) module instead.
-author: "Chris Church (@cchurch)"
+author:
+- Chris Church (@cchurch)
 '''
 
 EXAMPLES = r'''
@@ -52,7 +53,15 @@
 # Example from an Ansible Playbook
 - win_ping:
 
-# Induce a crash to see what happens
+# Induce an exception to see what happens
 - win_ping:
     data: crash
 '''
+
+RETURN = '''
+ping:
+    description: value provided with the data parameter
+    returned: success
+    type: string
+    sample: pong
+'''
diff --git a/test/integration/targets/ping/tasks/main.yml b/test/integration/targets/ping/tasks/main.yml
@@ -23,15 +23,31 @@
 - name: assert the ping worked
   assert:
     that:
-    - "result.changed == false"
-    - "result.ping == 'pong'"
+    - not result|failed
+    - not result|changed
+    - result.ping == 'pong'
 
 - name: ping with data
-  ping: data="testing"
+  ping:
+    data: testing
   register: result
 
 - name: assert the ping worked with data
   assert:
     that:
-    - "result.changed == false"
-    - "result.ping == 'testing'"
+    - not result|failed
+    - not result|changed
+    - result.ping == 'testing'
+
+- name: ping with data=crash
+  ping:
+    data: crash
+  register: result
+  ignore_errors: yes
+
+- name: assert the ping failed with data=boom
+  assert:
+    that:
+    - result|failed
+    - not result|changed
+    - "'Exception: boom' in result.module_stderr"
diff --git a/test/integration/targets/win_ping/tasks/main.yml b/test/integration/targets/win_ping/tasks/main.yml
@@ -23,20 +23,21 @@
 - name: check win_ping result
   assert:
     that:
-      - "not win_ping_result|failed"
-      - "not win_ping_result|changed"
-      - "win_ping_result.ping == 'pong'"
+      - not win_ping_result|failed
+      - not win_ping_result|changed
+      - win_ping_result.ping == 'pong'
 
 - name: test win_ping with data
-  win_ping: data=☠
+  win_ping:
+    data: ☠
   register: win_ping_with_data_result
 
 - name: check win_ping result with data
   assert:
     that:
-      - "not win_ping_with_data_result|failed"
-      - "not win_ping_with_data_result|changed"
-      - "win_ping_with_data_result.ping == '☠'"
+      - not win_ping_with_data_result|failed
+      - not win_ping_with_data_result|changed
+      - win_ping_with_data_result.ping == '☠'
 
 - name: test win_ping.ps1 with data as complex args
   win_ping.ps1:
@@ -46,9 +47,9 @@
 - name: check win_ping.ps1 result with data
   assert:
     that:
-      - "not win_ping_ps1_result|failed"
-      - "not win_ping_ps1_result|changed"
-      - "win_ping_ps1_result.ping == 'bleep'"
+      - not win_ping_ps1_result|failed
+      - not win_ping_ps1_result|changed
+      - win_ping_ps1_result.ping == 'bleep'
 
 - name: test win_ping with extra args to verify that v2 module replacer escaping works as expected
   win_ping:
@@ -76,9 +77,22 @@
 - name: check that win_ping with extra args succeeds and ignores everything except data
   assert:
     that:
-      - "not win_ping_extra_args_result|failed"
-      - "not win_ping_extra_args_result|changed"
-      - "win_ping_extra_args_result.ping == 'bloop'"
+      - not win_ping_extra_args_result|failed
+      - not win_ping_extra_args_result|changed
+      - win_ping_extra_args_result.ping == 'bloop'
+
+- name: test win_ping using data=crash so that it throws an exception
+  win_ping:
+    data: crash
+  register: win_ping_crash_result
+  ignore_errors: yes
+
+- name: check win_ping_crash result
+  assert:
+    that:
+      - win_ping_crash_result|failed
+      - not win_ping_crash_result|changed
+      - "'FullyQualifiedErrorId : boom' in win_ping_crash_result.module_stderr"
 
 # TODO: fix code or tests? discrete error returns from PS are strange...
 
@@ -90,11 +104,11 @@
 #- name: check win_ping_throw result
 #  assert:
 #    that:
-#      - "win_ping_throw_result|failed"
-#      - "not win_ping_throw_result|changed"
-#      - "win_ping_throw_result.msg == 'MODULE FAILURE'"
-#      - "win_ping_throw_result.exception"
-#      - "win_ping_throw_result.error_record"
+#      - win_ping_throw_result|failed
+#      - not win_ping_throw_result|changed
+#      - win_ping_throw_result.msg == 'MODULE FAILURE'
+#      - win_ping_throw_result.exception
+#      - win_ping_throw_result.error_record
 #
 #- name: test modified win_ping that throws a string exception
 #  action: win_ping_throw_string
@@ -104,11 +118,11 @@
 #- name: check win_ping_throw_string result
 #  assert:
 #    that:
-#      - "win_ping_throw_string_result|failed"
-#      - "not win_ping_throw_string_result|changed"
-#      - "win_ping_throw_string_result.msg == 'no ping for you'"
-#      - "win_ping_throw_string_result.exception"
-#      - "win_ping_throw_string_result.error_record"
+#      - win_ping_throw_string_result|failed
+#      - not win_ping_throw_string_result|changed
+#      - win_ping_throw_string_result.msg == 'no ping for you'
+#      - win_ping_throw_string_result.exception
+#      - win_ping_throw_string_result.error_record
 #
 #- name: test modified win_ping that has a syntax error
 #  action: win_ping_syntax_error
@@ -118,10 +132,10 @@
 #- name: check win_ping_syntax_error result
 #  assert:
 #    that:
-#      - "win_ping_syntax_error_result|failed"
-#      - "not win_ping_syntax_error_result|changed"
-#      - "win_ping_syntax_error_result.msg"
-#      - "win_ping_syntax_error_result.exception"
+#      - win_ping_syntax_error_result|failed
+#      - not win_ping_syntax_error_result|changed
+#      - win_ping_syntax_error_result.msg
+#      - win_ping_syntax_error_result.exception
 #
 #- name: test modified win_ping that has an error that only surfaces when strict mode is on
 #  action: win_ping_strict_mode_error
@@ -131,10 +145,10 @@
 #- name: check win_ping_strict_mode_error result
 #  assert:
 #    that:
-#      - "win_ping_strict_mode_error_result|failed"
-#      - "not win_ping_strict_mode_error_result|changed"
-#      - "win_ping_strict_mode_error_result.msg"
-#      - "win_ping_strict_mode_error_result.exception"
+#      - win_ping_strict_mode_error_result|failed
+#      - not win_ping_strict_mode_error_result|changed
+#      - win_ping_strict_mode_error_result.msg
+#      - win_ping_strict_mode_error_result.exception
 #
 #- name: test modified win_ping to verify a Set-Attr fix
 #  action: win_ping_set_attr data="fixed"
@@ -143,6 +157,6 @@
 #- name: check win_ping_set_attr_result result
 #  assert:
 #    that:
-#      - "not win_ping_set_attr_result|failed"
-#      - "not win_ping_set_attr_result|changed"
-#      - "win_ping_set_attr_result.ping == 'fixed'"
+#      - not win_ping_set_attr_result|failed
+#      - not win_ping_set_attr_result|changed
+#      - win_ping_set_attr_result.ping == 'fixed'