Fix tests as filters #4 (#33930)

* Resolve newly added tests as filters

* Add code smell to test for ansible provided jinja tests as filters syntax

* Add docs for no-tests-as-filters code smell test

* Address tests as filters in new integration tests

* Address feedback

* Address feedback 2

docs/docsite/rst/dev_guide/testing/sanity/no-tests-as-filters.rst

@@ -0,0 +1,10 @@
+Sanity Tests » no-tests-as-filters
+==================================
+
+Using Ansible provided Jinja2 tests as filters will be removed in Ansible 2.9.
+
+Prior to Ansible 2.5, Jinja2 tests included within Ansible were most often used as filters. The large difference in use is that filters are referenced as ``variable | filter_name`` where as Jinja2 tests are refereced as ``variable is test_name``.
+
+Jinja2 tests are used for comparisons, whereas filters are used for data manipulation, and have different applications in Jinja2. This change is to help differentiate the concepts for a better understanding of Jinja2, and where each can be appropriately used.
+
+As of Ansible 2.5 using an Ansible provided Jinja2 test with filter syntax will display a deprecation error.

test/integration/targets/nxos_gir_profile_management/tests/common/sanity.yaml

@@ -87,7 +87,7 @@
 
   - assert: *false
 
-  when: not (platform | match('N35')) and not titanium
+  when: not ( platform is match('N35')) and not titanium
 
   rescue:
 

test/integration/targets/vmware_host/tasks/main.yml

@@ -83,7 +83,7 @@
 - name: ensure host system is present
   assert:
     that:
-        - add_host_result | changed
+        - add_host_result is changed
         - "{% for host in host_list.json if ((host | basename) == 'test_host_system_0001') -%} True {%- else -%} False {%- endfor %}"
 
 # Testcase: Add Host again
@@ -104,7 +104,7 @@
 - name: ensure precend task didn't changed anything
   assert:
     that:
-        - not (readd_host_result|changed)
+        - not ( readd_host_result is changed)
 
 # Testcase: Add Host via add_or_reconnect state
 - name: add host via add_or_reconnect
@@ -129,7 +129,7 @@
 - name: ensure host system is present
   assert:
     that:
-        - add_or_reconnect_host_result | changed
+        - add_or_reconnect_host_result is changed
         - "{% for host in host_list.json if ((host | basename) == 'test_host_system_0002') -%} True {%- else -%} False {%- endfor %}"
 
 ## Testcase: Reconnect Host
@@ -152,7 +152,7 @@
 #- name: ensure host system has been reconnected
 #  assert:
 #    that:
-#        - reconnect_host_result | changed
+#        - reconnect_host_result is changed
 #        # it would be a good idea to check the events on the host to see the reconnect
 #        # https://github.com/vmware/govmomi/blob/master/govc/USAGE.md#events
 #        # "govc events ..." need to be callable from
@@ -183,7 +183,7 @@
 #- name: ensure host system is absent
 #  assert:
 #    that:
-#        - remove_host_result | changed
+#        - remove_host_result is changed
 #        - "{% for host in host_list.json if ((host | basename) == 'test_host_system_0001') -%} False {%- else -%} True {%- endfor %}"
 
 ## Testcase: Remove Host again
@@ -206,4 +206,4 @@
 #- name: ensure precend task didn't changed anything
 #  assert:
 #    that:
-#        - not (reremove_host_result|changed)
+#        - not ( reremove_host_result is changed)

test/integration/targets/win_disk_facts/tasks/main.yml

@@ -6,7 +6,7 @@
   ignore_errors: true
 
 - name: Only run tests when Windows is capable
-  when: (win_feature_has_storage_module|success) and (ansible_powershell_version is defined) and (ansible_powershell_version >= 3)
+  when: ( win_feature_has_storage_module is successful) and (ansible_powershell_version is defined) and (ansible_powershell_version >= 3)
   block:
 
   - name: Test in normal mode

test/legacy/roles/cloudscale_floating_ip/tasks/floating_ip.yml

@@ -8,8 +8,8 @@
 - name: Verify request floating IP
   assert:
     that:
-      - floating_ip | success
+      - floating_ip is successful
-      - floating_ip | changed
+      - floating_ip is changed
       - (item.ip_version == 4 and floating_ip.ip | ipv4) or (item.ip_version == 6 and floating_ip.ip | ipv6)
       - floating_ip.server == test01.uuid
 
@@ -21,8 +21,8 @@
 - name: Verify floating IP indempotence
   assert:
     that:
-      - floating_ip_indempotence | success
+      - floating_ip_indempotence is successful
-      - not floating_ip_indempotence | changed
+      - floating_ip_indempotence is not changed
       - floating_ip_indempotence.server == test01.uuid
 
 - name: Check network parameter alias
@@ -33,7 +33,7 @@
 - name: Verify network parameter alias
   assert:
     that:
-      - floating_ip_network | success
+      - floating_ip_network is successful
 
 - name: Move floating IP to second server
   cloudscale_floating_ip:
@@ -43,8 +43,8 @@
 - name: Verify move floating IPv4 to second server
   assert:
     that:
-      - move_ip | success
+      - move_ip is successful
-      - move_ip | changed
+      - move_ip is changed
       - move_ip.server == test02.uuid
 
 - name: Fail if server is missing on update
@@ -55,7 +55,7 @@
 - name: Verify fail if server is missing on update
   assert:
     that:
-      - update_failed | failed
+      - update_failed is failed
       - "'Missing required parameter' in update_failed.msg"
 
 - name: Release floating IP
@@ -66,8 +66,8 @@
 - name: Verify release floating IPs
   assert:
     that:
-      - release_ip | success
+      - release_ip is successful
-      - release_ip | changed
+      - release_ip is changed
       - release_ip.state == 'absent'
 
 - name: Release floating IP indempotence
@@ -78,8 +78,8 @@
 - name: Verify release floating IPs indempotence
   assert:
     that:
-      - release_ip | success
+      - release_ip is successful
-      - not release_ip | changed
+      - release_ip is not changed
       - release_ip.state == 'absent'
 
 - name: Fail if server is missing on request
@@ -90,5 +90,5 @@
 - name: Verify fail if server is missing on request
   assert:
     that:
-      - request_failed | failed
+      - request_failed is failed
       - "'Missing required parameter' in request_failed.msg"

test/sanity/code-smell/no-tests-as-filters.py

@@ -0,0 +1,90 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# (c) 2017, Matt Martz <matt@sivel.net>
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+from __future__ import print_function
+
+import os
+import re
+import sys
+
+from collections import defaultdict
+
+from ansible.plugins.test import core, files, mathstuff
+
+TESTS = list(core.TestModule().tests().keys()) + list(files.TestModule().tests().keys()) + list(mathstuff.TestModule().tests().keys())
+
+
+TEST_MAP = {
+    'version_compare': 'version',
+    'is_dir': 'directory',
+    'is_file': 'file',
+    'is_link': 'link',
+    'is_abs': 'abs',
+    'is_same_file': 'same_file',
+    'is_mount': 'mount',
+    'issubset': 'subset',
+    'issuperset': 'superset',
+    'isnan': 'nan',
+    'succeeded': 'successful',
+    'success': 'successful',
+    'change': 'changed',
+    'skip': 'skipped',
+}
+
+
+FILTER_RE = re.compile(r'((.+?)\s*([\w \.\'"]+)(\s*)\|(\s*)(\w+))')
+
+
+def main():
+    all_matches = defaultdict(list)
+
+    for root, dirs, filenames in os.walk('.'):
+        for name in filenames:
+            if os.path.splitext(name)[1] not in ('.yml', '.yaml'):
+                continue
+            path = os.path.join(root, name)
+
+            with open(path) as f:
+                text = f.read()
+
+            for match in FILTER_RE.findall(text):
+                filter_name = match[5]
+
+                try:
+                    test_name = TEST_MAP[filter_name]
+                except KeyError:
+                    test_name = filter_name
+
+                if test_name not in TESTS:
+                    continue
+
+                all_matches[path].append(match[0])
+
+    if all_matches:
+        print('Use of Ansible provided Jinja2 tests as filters is deprecated.')
+        print('Please update to use `is` syntax such as `result is failed`.')
+
+        for path, matches in all_matches.items():
+            for match in matches:
+                print('%s: %s' % (path, match,))
+        sys.exit(1)
+
+
+if __name__ == '__main__':
+    main()