snap - fixed param order (#2918) (#2932)

* fixed param order

* added changelog fragment

* rebased and uncommented tests per PR

* added /snap link in RH

* typo in tests

* Update tests/integration/targets/snap/tasks/default.yml

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 9b02230477)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

changelogs/fragments/2918-snap-param-order.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - snap - fixed the order of the ``--classic`` parameter in the command line invocation (https://github.com/ansible-collections/community.general/issues/2916).

plugins/modules/packaging/os/snap.py

@@ -133,10 +133,10 @@ class Snap(CmdStateModuleHelper):
     module = dict(
         argument_spec={
             'name': dict(type='list', elements='str', required=True),
-            'state': dict(type='str', required=False, default='present',
+            'state': dict(type='str', default='present',
                           choices=['absent', 'present', 'enabled', 'disabled']),
-            'classic': dict(type='bool', required=False, default=False),
+            'classic': dict(type='bool', default=False),
-            'channel': dict(type='str', required=False, default='stable'),
+            'channel': dict(type='str', default='stable'),
         },
         supports_check_mode=True,
     )
@@ -205,7 +205,7 @@ class Snap(CmdStateModuleHelper):
         self.vars.snaps_installed = actionable_snaps
         if self.module.check_mode:
             return
-        params = ['classic', 'channel', 'state']  # get base cmd parts
+        params = ['state', 'classic', 'channel']  # get base cmd parts
         has_one_pkg_params = bool(self.vars.classic) or self.vars.channel != 'stable'
         has_multiple_snaps = len(actionable_snaps) > 1
         if has_one_pkg_params and has_multiple_snaps:

tests/integration/targets/snap/tasks/default.yml

@@ -10,6 +10,12 @@
     name: snapd
     state: started
 
+- name: Create link /snap
+  file:
+    src: /var/lib/snapd/snap
+    dest: /snap
+    state: link
+
 - name: Inform that snap is installed
   set_fact:
     has_snap: true

tests/integration/targets/snap/tasks/main.yml

@@ -96,50 +96,50 @@
           - remove_again is not changed
           - remove_again_check is not changed
 
-#    - name: Make sure package from classic snap is not installed
+    - name: Make sure package from classic snap is not installed
-#      community.general.snap:
+      community.general.snap:
-#        name: nvim
+        name: nvim
-#        state: absent
+        state: absent
-#
+
-#    - name: Install package from classic snap
+    - name: Install package from classic snap
-#      community.general.snap:
+      community.general.snap:
-#        name: nvim
+        name: nvim
-#        state: present
+        state: present
-#        classic: true
+        classic: true
-#      register: classic_install
+      register: classic_install
-#
+
-#    # testing classic idempotency
+    # testing classic idempotency
-#    - name: Install package from classic snap again
+    - name: Install package from classic snap again
-#      community.general.snap:
+      community.general.snap:
-#        name: nvim
+        name: nvim
-#        state: present
+        state: present
-#        classic: true
+        classic: true
-#      register: classic_install_again
+      register: classic_install_again
-#
+
-#    - name: Assert package has been installed just once
+    - name: Assert package has been installed just once
-#      assert:
+      assert:
-#        that:
+        that:
-#          - classic_install is changed
+          - classic_install is changed
-#          - classic_install_again is not changed
+          - classic_install_again is not changed
-#
+
-#    # this is just testing if a package which has been installed
+    # this is just testing if a package which has been installed
-#    # with true classic can be removed without setting classic to true
+    # with true classic can be removed without setting classic to true
-#    - name: Remove package from classic snap without setting classic to true
+    - name: Remove package from classic snap without setting classic to true
-#      community.general.snap:
+      community.general.snap:
-#        name: nvim
+        name: nvim
-#        state: absent
+        state: absent
-#      register: classic_remove_without_true_classic
+      register: classic_remove_without_true_classic
-#
+
-#    - name: Remove package from classic snap with setting classic to true
+    - name: Remove package from classic snap with setting classic to true
-#      community.general.snap:
+      community.general.snap:
-#        name: nvim
+        name: nvim
-#        state: absent
+        state: absent
-#        classic: true
+        classic: true
-#      register: classic_remove_with_true_classic
+      register: classic_remove_with_true_classic
-#
+
-#    - name: Assert package has been removed without setting classic to true
+    - name: Assert package has been removed without setting classic to true
-#      assert:
+      assert:
-#        that:
+        that:
-#          - classic_remove_without_ture_classic is changed
+          - classic_remove_without_true_classic is changed
-#          - classic_remove_with_ture_classic is not changed
+          - classic_remove_with_true_classic is not changed
   when: has_snap