MH deco: minor refactor (ansible-collections#8766)

* MH deco: minor refactor

* adjustments and improvement in test

* sanity fix

* use func.__self__

* simplify use of self

* add changelog frag

changelogs/fragments/8766-mh-deco-improve.yml

@@ -0,0 +1,3 @@
+minor_changes:
+  - MH module utils - add parameter ``when`` to ``cause_changes`` decorator (https://github.com/ansible-collections/community.general/pull/8766).
+  - MH module utils - minor refactor in decorators (https://github.com/ansible-collections/community.general/pull/8766).

plugins/module_utils/mh/deco.py

@@ -13,23 +13,26 @@
 from ansible_collections.community.general.plugins.module_utils.mh.exceptions import ModuleHelperException
 
 
-def cause_changes(on_success=None, on_failure=None):
+def cause_changes(on_success=None, on_failure=None, when=None):
 
     def deco(func):
-        if on_success is None and on_failure is None:
-            return func
-
         @wraps(func)
-        def wrapper(*args, **kwargs):
+        def wrapper(self, *args, **kwargs):
             try:
-                self = args[0]
-                func(*args, **kwargs)
+                func(self, *args, **kwargs)
                 if on_success is not None:
                     self.changed = on_success
+                elif when == "success":
+                    self.changed = True
             except Exception:
                 if on_failure is not None:
                     self.changed = on_failure
+                elif when == "failure":
+                    self.changed = True
                 raise
+            finally:
+                if when == "always":
+                    self.changed = True
 
         return wrapper
 
@@ -50,8 +53,6 @@ def fix_var_conflicts(output):
 
         try:
             func(self, *args, **kwargs)
-        except SystemExit:
-            raise
         except ModuleHelperException as e:
             if e.update_output:
                 self.update_output(e.update_output)
@@ -73,6 +74,7 @@ def check_mode_skip(func):
     def wrapper(self, *args, **kwargs):
         if not self.module.check_mode:
             return func(self, *args, **kwargs)
+
     return wrapper
 
 
@@ -87,15 +89,12 @@ def wrapper_callable(self, *args, **kwargs):
                 return func(self, *args, **kwargs)
             return wrapper_callable
 
-        if value is not None:
+        else:
             @wraps(func)
             def wrapper_value(self, *args, **kwargs):
                 if self.module.check_mode:
                     return value
                 return func(self, *args, **kwargs)
             return wrapper_value
 
-    if callable is None and value is None:
-        return check_mode_skip
-
     return deco

tests/unit/plugins/module_utils/test_module_helper.py

@@ -119,28 +119,22 @@ def test_variable_meta_change():
     assert vd.has_changed('d')
 
 
-class MockMH(object):
-    changed = None
-
-    def _div(self, x, y):
-        return x / y
-
-    func_none = cause_changes()(_div)
-    func_onsucc = cause_changes(on_success=True)(_div)
-    func_onfail = cause_changes(on_failure=True)(_div)
-    func_onboth = cause_changes(on_success=True, on_failure=True)(_div)
-
-
-CAUSE_CHG_DECO_PARAMS = ['method', 'expect_exception', 'expect_changed']
+CAUSE_CHG_DECO_PARAMS = ['deco_args', 'expect_exception', 'expect_changed']
 CAUSE_CHG_DECO = dict(
-    none_succ=dict(method='func_none', expect_exception=False, expect_changed=None),
-    none_fail=dict(method='func_none', expect_exception=True, expect_changed=None),
-    onsucc_succ=dict(method='func_onsucc', expect_exception=False, expect_changed=True),
-    onsucc_fail=dict(method='func_onsucc', expect_exception=True, expect_changed=None),
-    onfail_succ=dict(method='func_onfail', expect_exception=False, expect_changed=None),
-    onfail_fail=dict(method='func_onfail', expect_exception=True, expect_changed=True),
-    onboth_succ=dict(method='func_onboth', expect_exception=False, expect_changed=True),
-    onboth_fail=dict(method='func_onboth', expect_exception=True, expect_changed=True),
+    none_succ=dict(deco_args={}, expect_exception=False, expect_changed=None),
+    none_fail=dict(deco_args={}, expect_exception=True, expect_changed=None),
+    onsucc_succ=dict(deco_args=dict(on_success=True), expect_exception=False, expect_changed=True),
+    onsucc_fail=dict(deco_args=dict(on_success=True), expect_exception=True, expect_changed=None),
+    onfail_succ=dict(deco_args=dict(on_failure=True), expect_exception=False, expect_changed=None),
+    onfail_fail=dict(deco_args=dict(on_failure=True), expect_exception=True, expect_changed=True),
+    onboth_succ=dict(deco_args=dict(on_success=True, on_failure=True), expect_exception=False, expect_changed=True),
+    onboth_fail=dict(deco_args=dict(on_success=True, on_failure=True), expect_exception=True, expect_changed=True),
+    whensucc_succ=dict(deco_args=dict(when="success"), expect_exception=False, expect_changed=True),
+    whensucc_fail=dict(deco_args=dict(when="success"), expect_exception=True, expect_changed=None),
+    whenfail_succ=dict(deco_args=dict(when="failure"), expect_exception=False, expect_changed=None),
+    whenfail_fail=dict(deco_args=dict(when="failure"), expect_exception=True, expect_changed=True),
+    whenalways_succ=dict(deco_args=dict(when="always"), expect_exception=False, expect_changed=True),
+    whenalways_fail=dict(deco_args=dict(when="always"), expect_exception=True, expect_changed=True),
 )
 CAUSE_CHG_DECO_IDS = sorted(CAUSE_CHG_DECO.keys())
 
@@ -150,12 +144,20 @@ def _div(self, x, y):
                           for param in CAUSE_CHG_DECO_PARAMS]
                           for tc in CAUSE_CHG_DECO_IDS],
                          ids=CAUSE_CHG_DECO_IDS)
-def test_cause_changes_deco(method, expect_exception, expect_changed):
+def test_cause_changes_deco(deco_args, expect_exception, expect_changed):
+
+    class MockMH(object):
+        changed = None
+
+        @cause_changes(**deco_args)
+        def div_(self, x, y):
+            return x / y
+
     mh = MockMH()
     if expect_exception:
         with pytest.raises(Exception):
-            getattr(mh, method)(1, 0)
+            mh.div_(1, 0)
     else:
-        getattr(mh, method)(9, 3)
+        mh.div_(9, 3)
 
     assert mh.changed == expect_changed