collections - working unit tests during integration

The unit tests that are run during integration test did not
work for the role converted to collection format.  The tests need to
get the paths from the environment then set up the runtime environment
to look like the real Ansible runtime environment.

Signed-off-by: Rich Megginson <rmeggins@redhat.com>

pytest_extra_requirements.txt

@@ -5,3 +5,8 @@
 #-ransible_pytest_extra_requirements.txt
 # If you need mock then uncomment the following line:
 mock ; python_version < "3.0"
+# ansible and dependencies for all supported platforms
+ansible ; python_version > "2.6"
+ansible<2.7 ; python_version < "2.7"
+idna<2.8 ; python_version < "2.7"
+PyYAML<5.1 ; python_version < "2.7"

tests/integration/test_ethernet.py

@@ -1,32 +1,25 @@
 # -*- coding: utf-8 -*
 # SPDX-License-Identifier: BSD-3-Clause
-
 import logging
 import os
-import pytest
 import subprocess
-import sys
+
+import pytest
 
 try:
     from unittest import mock
 except ImportError:
     import mock
 
-parentdir = os.path.normpath(os.path.join(os.path.dirname(__file__), "../../"))
-with mock.patch.object(
-    sys,
-    "path",
-    [parentdir, os.path.join(parentdir, "module_utils/network_lsr")] + sys.path,
+parent_dir = os.path.normpath(os.path.join(os.path.dirname(__file__), "..", ".."))
+
+with mock.patch.dict(
+    "sys.modules",
+    {
+        "ansible.module_utils.basic": mock.Mock(),
+    },
 ):
-    with mock.patch.dict(
-        "sys.modules",
-        {
-            "ansible": mock.Mock(),
-            "ansible.module_utils": __import__("module_utils"),
-            "ansible.module_utils.basic": mock.Mock(),
-        },
-    ):
-        import library.network_connections as nc
+    import network_connections as nc
 
 
 class PytestRunEnvironment(nc.RunEnvironment):
@@ -94,20 +87,13 @@ def _get_ip_addresses(interface):
 
 @pytest.fixture
 def network_lsr_nm_mock():
-    with mock.patch.object(
-        sys,
-        "path",
-        [parentdir, os.path.join(parentdir, "module_utils/network_lsr/nm")] + sys.path,
+    with mock.patch.dict(
+        "sys.modules",
+        {
+            "ansible.module_utils.basic": mock.Mock(),
+        },
     ):
-        with mock.patch.dict(
-            "sys.modules",
-            {
-                "ansible": mock.Mock(),
-                "ansible.module_utils": __import__("module_utils"),
-                "ansible.module_utils.basic": mock.Mock(),
-            },
-        ):
-            yield
+        yield
 
 
 def test_static_ip_with_ethernet(testnic1, provider, network_lsr_nm_mock):

tests/playbooks/integration_pytest_python3.yml

@@ -29,64 +29,121 @@
 
 - name: Run Pytest tests
   hosts: all
-  vars:
-    - rundir: /run/system-roles-test
   tasks:
-    - file:
-        state: directory
-        path: "{{ rundir }}"
-        recurse: true
-
-    - command: git rev-parse --show-toplevel
-      register: git_top_directory
-      delegate_to: localhost
-
-    - debug:
-        var: git_top_directory
-
-    - synchronize:
-        src: "{{ git_top_directory.stdout }}/"
-        dest: "{{ rundir }}/"
-        recursive: yes
-        delete: yes
-        rsync_opts:
-          - "--exclude=.pyc"
-          - "--exclude=__pycache__"
-      when: False
-
-    # TODO: using tar and copying the file is a workaround for the synchronize
-    # module that does not work in test-harness. Related issue:
-    # https://github.com/linux-system-roles/test-harness/issues/102
-    #
-    - name: Create Tar file
-      shell: 'tar -cvf {{ git_top_directory.stdout }}/testrepo.tar
-             --exclude "*.pyc" --exclude "__pycache__" --exclude testrepo.tar
-             -C {{ git_top_directory.stdout }} .'
-      delegate_to: localhost
-
-    - name: Copy testrepo.tar to the remote system
-      copy:
-        src: "{{ git_top_directory.stdout }}/testrepo.tar"
-        dest: "{{ rundir }}"
-
-    - name: Untar testrepo.tar
-      command: tar xf testrepo.tar
-      args:
-        chdir: "{{ rundir }}"
-
     - block:
-        - name: Run pytest with nm
-          command: "pytest {{ rundir }}/tests/integration/ --provider=nm"
-          register: playbook_run
-      always:
-        - debug:
-            var: playbook_run.stdout_lines
+        - name: create tempdir for code to test
+          tempfile:
+            state: directory
+            prefix: lsrtest_
+          register: _rundir
 
-    - block:
-        - name: Run pytest with initscripts
+        - name: get tempfile for tar
+          tempfile:
+            prefix: lsrtest_
+            suffix: ".tar"
+          register: temptar
+          delegate_to: localhost
+
+        - include_tasks: ../tasks/get_modules_and_utils_paths.yml
+
+        - name: get tests directory
+          set_fact:
+            tests_directory: "{{ lookup('first_found', params) }}"
+          vars:
+            params:
+              files:
+                - tests
+                - network
+              paths:
+                - "../.."
+
+        # TODO: using tar and copying the file is a workaround for the
+        # synchronize module that does not work in test-harness. Related issue:
+        # https://github.com/linux-system-roles/test-harness/issues/102
+        #
+        - name: Create Tar file
           command: >
-            pytest '{{ rundir }}/tests/integration/' --provider=initscripts
-          register: playbook_run
-      always:
+            tar -cvf {{ temptar.path }} --exclude "*.pyc"
+                --exclude "__pycache__"
+                -C {{ tests_directory | realpath | dirname }}
+                {{ tests_directory | basename }}
+                -C {{ modules_parent_and_dir.stdout_lines[0] }}
+                {{ modules_parent_and_dir.stdout_lines[1] }}
+                -C {{ module_utils_parent_and_dir.stdout_lines[0] }}
+                {{ module_utils_parent_and_dir.stdout_lines[1] }}
+          delegate_to: localhost
+
+        - name: Copy testrepo.tar to the remote system
+          copy:
+            src: "{{ temptar.path }}"
+            dest: "{{ _rundir.path }}"
+
+        - name: Untar testrepo.tar
+          command: tar xf {{ temptar.path | basename }}
+          args:
+            chdir: "{{ _rundir.path }}"
+
+        - file:
+            state: directory
+            path: "{{ _rundir.path }}/ansible"
+
+        - name: Move module_utils to ansible directory
+          shell: |
+            if [ -d {{ _rundir.path }}/module_utils ]; then
+              mv {{ _rundir.path }}/module_utils {{ _rundir.path }}/ansible
+            fi
+
+        - name: Fake out python module directories, primarily for python2
+          shell: |
+            for dir in $(find {{ _rundir.path }} -type d -print); do
+              if [ ! -f "$dir/__init__.py" ]; then
+                touch "$dir/__init__.py"
+              fi
+            done
+
+        - set_fact:
+            _lsr_python_path: "{{
+                  _rundir.path ~ '/' ~
+                  modules_parent_and_dir.stdout_lines[1] ~ ':' ~ _rundir.path
+            }}"
+
         - debug:
-            var: playbook_run.stdout_lines
+            msg: path {{ _lsr_python_path }}
+        - command: ls -alrtFR {{ _rundir.path }}
+
+        - block:
+            - name: Run pytest with nm
+              command: >
+                pytest
+                {{ _rundir.path }}/{{ tests_directory | basename }}/integration/
+                --provider=nm
+              register: playbook_run
+              environment:
+                PYTHONPATH: "{{ _lsr_python_path }}"
+          always:
+            - debug:
+                var: playbook_run.stdout_lines
+
+        - block:
+            - name: Run pytest with initscripts
+              command: >
+                pytest
+                {{ _rundir.path }}/{{ tests_directory | basename }}/integration/
+                --provider=initscripts
+              register: playbook_run
+              environment:
+                PYTHONPATH: "{{ _lsr_python_path }}"
+          always:
+            - debug:
+                var: playbook_run.stdout_lines
+      always:
+        - name: remove local tar file
+          file:
+            state: absent
+            path: "{{ temptar.path }}"
+          delegate_to: localhost
+
+        - name: remove tempdir
+          file:
+            state: absent
+            path: "{{ _rundir.path }}"

tests/tasks/get_modules_and_utils_paths.yml

@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: BSD-3-Clause
+---
+- name: set collection paths
+  set_fact:
+    collection_paths: |
+      {{
+        (lookup("env","ANSIBLE_COLLECTIONS_PATH").split(":") +
+        lookup("env","ANSIBLE_COLLECTIONS_PATHS").split(":") +
+        lookup("config", "COLLECTIONS_PATHS")) |
+        select | list
+      }}
+
+- name: set search paths
+  set_fact:
+    modules_search_path: |
+        {{
+          (lookup("env", "ANSIBLE_LIBRARY").split(":") +
+          ["../../library", "../library"] +
+          lookup("config", "DEFAULT_MODULE_PATH")) |
+          select | list
+        }}
+    module_utils_search_path: |
+        {{
+          (lookup("env", "ANSIBLE_MODULE_UTILS").split(":") +
+          ["../../module_utils", "../module_utils"] +
+          lookup("config", "DEFAULT_MODULE_UTILS_PATH")) |
+          select | list
+        }}
+
+# the output should be something like
+# - path to parent directory to chdir to in order to use tar
+# - relative path under parent directory to tar
+# e.g. for the local role case
+# - ../..
+# - library
+# would translate to tar -C ../.. library
+# for the collection case
+# - /home/user/.ansible/collections
+# - ansible_collections/fedora/linux_system_roles/plugins/modules
+# would translate to tar -C /home/user/.ansible/collections \
+# ansible_collections/fedora/linux_system_roles/plugins/modules
+- name: find parent directory and path of modules
+  shell: |
+    set -euxo pipefail
+    for dir in {{ modules_search_path | join(" ") }}; do
+      if [ -f "$dir/network_connections.py" ]; then
+        readlink -f "$(dirname "$dir")"
+        basename "$dir"
+        exit 0
+      fi
+    done
+    for dir in {{ collection_paths | join(" ") }}; do
+      cd "$dir"
+      for subdir in ansible_collections/*/*/plugins/modules; do
+        if [ -f "$subdir/network_connections.py" ]; then
+          echo "$dir"
+          echo "$subdir"
+          exit 0
+        fi
+      done
+    done
+    echo network_connections.py not found
+    exit 1
+  delegate_to: localhost
+  register: modules_parent_and_dir
+
+- name: find parent directory and path of module_utils
+  shell: |
+    set -euxo pipefail
+    for dir in {{ module_utils_search_path | join(" ") }}; do
+      if [ -d "$dir/network_lsr" ]; then
+        readlink -f "$(dirname "$dir")"
+        basename "$dir"
+        exit 0
+      fi
+    done
+    for dir in {{ collection_paths | join(" ") }}; do
+      cd "$dir"
+      for subdir in ansible_collections/*/*/plugins/module_utils; do
+        if [ -d "$subdir/network_lsr" ]; then
+          echo "$dir"
+          echo "$subdir"
+          exit 0
+        fi
+      done
+    done
+    echo network_lsr not found
+    exit 1
+  delegate_to: localhost
+  register: module_utils_parent_and_dir

tests/tests_unit.yml

@@ -21,60 +21,137 @@
 - hosts: all
   name: execute python unit tests
   tasks:
-    - name: Copy python modules
-      copy:
-        src: "{{ item }}"
-        dest: /tmp/test-unit-1/
-        local_follow: false
-      loop:
-        - ../library/network_connections.py
-        - unit/test_network_connections.py
-        - ../module_utils/network_lsr
+    - block:
+        - name: create tempdir for code to test
+          tempfile:
+            state: directory
+            prefix: lsrtest_
+          register: _rundir
 
-    - name: Create helpers directory
-      file:
-        state: directory
-        dest: /tmp/test-unit-1/helpers
+        - name: get tempfile for tar
+          tempfile:
+            prefix: lsrtest_
+            suffix: ".tar"
+          register: temptar
+          delegate_to: localhost
 
-    - name: Copy helpers
-      copy:
-        src: "{{ item }}"
-        dest: /tmp/test-unit-1/helpers
-        mode: 0755
-      with_fileglob:
-        - unit/helpers/*
+        - include_tasks: tasks/get_modules_and_utils_paths.yml
 
-    - name: Check if python2 is available
-      command: python2 --version
-      ignore_errors: true
-      register: python2_available
-      when: true
+        # TODO: using tar and copying the file is a workaround for the
+        # synchronize module that does not work in test-harness. Related issue:
+        # https://github.com/linux-system-roles/test-harness/issues/102
+        #
+        - name: Create Tar file
+          command: >
+            tar -cvf {{ temptar.path }} --exclude "*.pyc"
+                --exclude "__pycache__"
+                -C {{ modules_parent_and_dir.stdout_lines[0] }}
+                {{ modules_parent_and_dir.stdout_lines[1] }}
+                -C {{ module_utils_parent_and_dir.stdout_lines[0] }}
+                {{ module_utils_parent_and_dir.stdout_lines[1] }}
+          delegate_to: localhost
 
-    - name: Run python2 unit tests
-      command: python2 /tmp/test-unit-1/test_network_connections.py --verbose
-      when: python2_available is succeeded and ansible_distribution != 'Fedora'
-      register: python2_result
+        - name: Copy testrepo.tar to the remote system
+          copy:
+            src: "{{ temptar.path }}"
+            dest: "{{ _rundir.path }}"
 
-    - name: Check if python3 is available
-      command: python3 --version
-      ignore_errors: true
-      register: python3_available
-      when: true
+        - name: Untar testrepo.tar
+          command: tar -xvf {{ temptar.path | basename }}
+          args:
+            chdir: "{{ _rundir.path }}"
 
-    - name: Run python3 unit tests
-      command: python3 /tmp/test-unit-1/test_network_connections.py --verbose
-      when: python3_available is succeeded
-      register: python3_result
+        - file:
+            state: directory
+            path: "{{ item }}"
+          loop:
+            - "{{ _rundir.path }}/ansible"
+            - "{{ _rundir.path }}/ansible/module_utils"
 
-    - name: Show python2 unit test results
-      debug:
-        var: python2_result.stderr_lines
-      when: python2_result is succeeded
+        - name: Move module_utils to ansible directory
+          shell: |
+            if [ -d {{ _rundir.path }}/module_utils ]; then
+              mv {{ _rundir.path }}/module_utils {{ _rundir.path }}/ansible
+            fi
 
-    - name: Show python3 unit test results
-      debug:
-        var: python3_result.stderr_lines
-      when: python3_result is succeeded
+        - name: Fake out python module directories, primarily for python2
+          shell: |
+            for dir in $(find {{ _rundir.path }} -type d -print); do
+              if [ ! -f "$dir/__init__.py" ]; then
+                touch "$dir/__init__.py"
+              fi
+            done
+
+        - name: Copy unit test to remote system
+          copy:
+            src: unit/test_network_connections.py
+            dest: "{{ _rundir.path }}"
+
+        - set_fact:
+            _lsr_python_path: "{{
+              _rundir.path ~ '/' ~
+              modules_parent_and_dir.stdout_lines[1] ~ ':' ~
+              _rundir.path ~ '/' ~ 'ansible' ~ '/' ~
+              module_utils_parent_and_dir.stdout_lines[1] ~ ':' ~
+              _rundir.path ~ '/' ~
+              module_utils_parent_and_dir.stdout_lines[1] ~ ':' ~
+              _rundir.path
+            }}"
+
+        - command: ls -alrtFR {{ _rundir.path }}
+        - debug:
+            msg: path {{ _lsr_python_path }}
+
+        - name: Check if python2 is available
+          command: python2 --version
+          ignore_errors: true
+          register: python2_available
+          when: true
+
+        - name: Run python2 unit tests
+          command: >
+            python2 {{ _rundir.path }}/test_network_connections.py --verbose
+          environment:
+            PYTHONPATH: "{{ _lsr_python_path }}"
+          when: >
+            python2_available is succeeded and ansible_distribution != 'Fedora'
+          register: python2_result
+
+        - name: Check if python3 is available
+          command: python3 --version
+          ignore_errors: true
+          register: python3_available
+          when: true
+
+        - name: Run python3 unit tests
+          command: >
+            python3 {{ _rundir.path }}/test_network_connections.py --verbose
+          environment:
+            PYTHONPATH: "{{ _lsr_python_path }}"
+          when: python3_available is succeeded
+          register: python3_result
+
+        - name: Show python2 unit test results
+          debug:
+            var: python2_result.stderr_lines
+          when: python2_result is succeeded
+
+        - name: Show python3 unit test results
+          debug:
+            var: python3_result.stderr_lines
+          when: python3_result is succeeded
+
+      always:
+        - name: remove local tar file
+          file:
+            state: absent
+            path: "{{ temptar.path }}"
+          delegate_to: localhost
+
+        - name: remove tempdir
+          file:
+            state: absent
+            path: "{{ _rundir.path }}"
 
     - name: Ensure that at least one python unit test ran
       fail:

tests/unit/test_network_connections.py

@@ -1,36 +1,26 @@
 #!/usr/bin/env python
 """ Tests for network_connections Ansible module """
 # SPDX-License-Identifier: BSD-3-Clause
-
+import copy
 import itertools
-import os
 import pprint as pprint_
 import socket
 import sys
 import unittest
-import copy
-
-TESTS_BASEDIR = os.path.dirname(os.path.abspath(__file__))
-sys.path.insert(1, os.path.join(TESTS_BASEDIR, "../..", "library"))
-sys.path.insert(1, os.path.join(TESTS_BASEDIR, "../..", "module_utils"))
 
 try:
     from unittest import mock
 except ImportError:  # py2
     import mock
 
-sys.modules["ansible"] = mock.Mock()
 sys.modules["ansible.module_utils.basic"] = mock.Mock()
-sys.modules["ansible.module_utils"] = mock.Mock()
-sys.modules["ansible.module_utils.network_lsr"] = __import__("network_lsr")
 
 # pylint: disable=import-error, wrong-import-position
+
 import network_lsr
-import network_connections as n
-
-from network_connections import SysUtil
-from network_connections import Util
-
+import network_lsr.argument_validator
+from network_connections import IfcfgUtil, NMUtil, SysUtil, Util
+from network_lsr.argument_validator import ValidationError
 
 try:
     my_test_skipIf = unittest.skipIf
@@ -44,7 +34,7 @@ except AttributeError:
 
 
 try:
-    nmutil = n.NMUtil()
+    nmutil = NMUtil()
     assert nmutil
 except Exception:
     # NMUtil is not supported, for example on RHEL 6 or without
@@ -52,8 +42,8 @@ except Exception:
     nmutil = None
 
 if nmutil:
-    NM = n.Util.NM()
-    GObject = n.Util.GObject()
+    NM = Util.NM()
+    GObject = Util.GObject()
 
 
 def pprint(msg, obj):
@@ -194,7 +184,7 @@ class TestValidator(unittest.TestCase):
         }
 
     def assertValidationError(self, v, value):
-        self.assertRaises(n.ValidationError, v.validate, value)
+        self.assertRaises(ValidationError, v.validate, value)
 
     def assert_nm_connection_routes_expected(self, connection, route_list_expected):
         parser = network_lsr.argument_validator.ArgValidatorIPRoute("route[?]")
@@ -229,13 +219,13 @@ class TestValidator(unittest.TestCase):
         for connection in connections:
             if "type" in connection:
                 connection["nm.exists"] = False
-                connection["nm.uuid"] = n.Util.create_uuid()
+                connection["nm.uuid"] = Util.create_uuid()
 
         mode = VALIDATE_ONE_MODE_NM
         for idx, connection in enumerate(connections):
             try:
                 ARGS_CONNECTIONS.validate_connection_one(mode, connections, idx)
-            except n.ValidationError:
+            except ValidationError:
                 continue
             if "type" in connection:
                 con_new = nmutil.connection_create(connections, idx)
@@ -278,7 +268,7 @@ class TestValidator(unittest.TestCase):
         for idx, connection in enumerate(connections):
             try:
                 ARGS_CONNECTIONS.validate_connection_one(mode, connections, idx)
-            except n.ValidationError:
+            except ValidationError:
                 continue
             if "type" not in connection:
                 continue
@@ -291,7 +281,7 @@ class TestValidator(unittest.TestCase):
             content_current = kwargs.get("initscripts_content_current", None)
             if content_current:
                 content_current = content_current[idx]
-            c = n.IfcfgUtil.ifcfg_create(
+            c = IfcfgUtil.ifcfg_create(
                 connections, idx, content_current=content_current
             )
             # pprint("con[%s] = \"%s\"" % (idx, connections[idx]['name']), c)
@@ -2477,7 +2467,7 @@ class TestValidator(unittest.TestCase):
         connections = ARGS_CONNECTIONS.validate(input_connections)
 
         self.assertRaises(
-            n.ValidationError,
+            ValidationError,
             ARGS_CONNECTIONS.validate_connection_one,
             VALIDATE_ONE_MODE_INITSCRIPTS,
             connections,
@@ -2526,7 +2516,7 @@ class TestValidator(unittest.TestCase):
         connections = ARGS_CONNECTIONS.validate(input_connections)
 
         self.assertRaises(
-            n.ValidationError,
+            ValidationError,
             ARGS_CONNECTIONS.validate_connection_one,
             VALIDATE_ONE_MODE_INITSCRIPTS,
             connections,
@@ -2672,7 +2662,7 @@ class TestValidator(unittest.TestCase):
             {"name": "internal_network", "type": "ethernet", "interface_name": None}
         ]
         self.assertRaises(
-            n.ValidationError, ARGS_CONNECTIONS.validate, network_connections
+            ValidationError, ARGS_CONNECTIONS.validate, network_connections
         )
 
     def test_interface_name_ethernet_explicit(self):
@@ -2688,7 +2678,7 @@ class TestValidator(unittest.TestCase):
         valid interface_name"""
         network_connections = [{"name": "internal:main", "type": "ethernet"}]
         self.assertRaises(
-            n.ValidationError, ARGS_CONNECTIONS.validate, network_connections
+            ValidationError, ARGS_CONNECTIONS.validate, network_connections
         )
         network_connections = [
             {"name": "internal:main", "type": "ethernet", "interface_name": "eth0"}
@@ -2701,7 +2691,7 @@ class TestValidator(unittest.TestCase):
             {"name": "internal", "type": "ethernet", "interface_name": "invalid:name"}
         ]
         self.assertRaises(
-            n.ValidationError, ARGS_CONNECTIONS.validate, network_connections
+            ValidationError, ARGS_CONNECTIONS.validate, network_connections
         )
 
     def test_interface_name_bond_empty_interface_name(self):
@@ -2709,7 +2699,7 @@ class TestValidator(unittest.TestCase):
             {"name": "internal", "type": "bond", "interface_name": "invalid:name"}
         ]
         self.assertRaises(
-            n.ValidationError, ARGS_CONNECTIONS.validate, network_connections
+            ValidationError, ARGS_CONNECTIONS.validate, network_connections
         )
 
     def test_interface_name_bond_profile_as_interface_name(self):
@@ -2745,19 +2735,19 @@ class TestValidator(unittest.TestCase):
     def test_invalid_persistent_state_up(self):
         network_connections = [{"name": "internal", "persistent_state": "up"}]
         self.assertRaises(
-            n.ValidationError, ARGS_CONNECTIONS.validate, network_connections
+            ValidationError, ARGS_CONNECTIONS.validate, network_connections
         )
 
     def test_invalid_persistent_state_down(self):
         network_connections = [{"name": "internal", "persistent_state": "down"}]
         self.assertRaises(
-            n.ValidationError, ARGS_CONNECTIONS.validate, network_connections
+            ValidationError, ARGS_CONNECTIONS.validate, network_connections
         )
 
     def test_invalid_state_test(self):
         network_connections = [{"name": "internal", "state": "test"}]
         self.assertRaises(
-            n.ValidationError, ARGS_CONNECTIONS.validate, network_connections
+            ValidationError, ARGS_CONNECTIONS.validate, network_connections
         )
 
     def test_default_states_type(self):