Fix collection loader and add unit tests. (#58701)

* Use `compile` before `eval` in collection loader.

This fixes two issues:

1. File names are available when tracing execution, such as with code coverage.
2. Future statements are not inherited from the collection loader.

* Add unit tests for collection loading.

These tests verify several things:

1. That unit tests can import code from collections when the collection loader is installed.
2. That tracing reports the correct file and line numbers (to support code coverage).
3. That collection code does not inherit __future__ statements from the collection loader.

* Update unit test handling of the collection loader.

Since the collection loader is installed simply by importing ansible.plugins.loader,
we may already have a collection loader installed when the test runs. This occurs if
any other tests are collected which use that import during collection. Until that code
is moved into an initialization function to avoid loading during import, the unit tests
will need to replace any existing collection loaders so that they reflect the desired
configuration.

* Insert into sys.modules before calling exec.

This is a requirement of PEP 302.

It will prevent recursion errors when importing the current module or using a relative import.

* Use the correct value for __package__ in modules.

This allows using relative imports in collections.

* Add warning about modifying code for trace test.

* Add test for relative import in collection.

* Add __init__.py to collection to satisfy pylint.

The relative-beyond-top-level rule in pylint may not be appropriate for collections.
However, until that rule is disabled for collections this will keep tests passing.

lib/ansible/utils/collection_loader.py

@@ -145,7 +145,7 @@ class AnsibleCollectionLoader(object):
             package_paths = [self._extend_path_with_ns(p, fullname) for p in parent_pkg.__path__]
 
         for candidate_child_path in package_paths:
-            source = None
+            code_object = None
             is_package = True
             location = None
             # check for implicit sub-package first
@@ -162,6 +162,8 @@ class AnsibleCollectionLoader(object):
 
                     with open(source_path, 'rb') as fd:
                         source = fd.read()
+
+                    code_object = compile(source=source, filename=source_path, mode='exec', flags=0, dont_inherit=True)
                     location = source_path
                     is_package = source_path.endswith('__init__.py')
                     break
@@ -170,7 +172,6 @@ class AnsibleCollectionLoader(object):
                     continue
 
             newmod = ModuleType(fullname)
-            newmod.__package__ = fullname
             newmod.__file__ = location
             newmod.__loader__ = self
 
@@ -180,12 +181,16 @@ class AnsibleCollectionLoader(object):
                 else:
                     newmod.__path__ = package_paths
 
-            if source:
-                # FIXME: decide cases where we don't actually want to exec the code?
-                exec(source, newmod.__dict__)
+                newmod.__package__ = fullname
+            else:
+                newmod.__package__ = parent_pkg_name
 
             sys.modules[fullname] = newmod
 
+            if code_object:
+                # FIXME: decide cases where we don't actually want to exec the code?
+                exec(code_object, newmod.__dict__)
+
             return newmod
 
         # FIXME: need to handle the "no dirs present" case for at least the root and synthetic internal collections like ansible.builtin

test/units/utils/fixtures/collections/ansible_collections/my_namespace/my_collection/plugins/action/my_action.py

@@ -0,0 +1 @@
+from ..module_utils.my_util import question

test/units/utils/fixtures/collections/ansible_collections/my_namespace/my_collection/plugins/module_utils/my_other_util.py

@@ -0,0 +1 @@
+from .my_util import question

test/units/utils/fixtures/collections/ansible_collections/my_namespace/my_collection/plugins/module_utils/my_util.py

@@ -0,0 +1,6 @@
+# WARNING: Changing line numbers of code in this file will break collection tests that use tracing to check paths and line numbers.
+#          Also, do not import division from __future__ as this will break detection of __future__ inheritance on Python 2.
+
+
+def question():
+    return 3 / 2

test/units/utils/test_collection_loader.py

@@ -0,0 +1,86 @@
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import os
+import sys
+
+
+def test_import_from_collection(monkeypatch):
+    collection_root = os.path.join(os.path.dirname(__file__), 'fixtures', 'collections')
+    collection_path = os.path.join(collection_root, 'ansible_collections/my_namespace/my_collection/plugins/module_utils/my_util.py')
+
+    # the trace we're expecting to be generated when running the code below:
+    # answer = question()
+    expected_trace_log = [
+        (collection_path, 5, 'call'),
+        (collection_path, 6, 'line'),
+        (collection_path, 6, 'return'),
+    ]
+
+    # define the collection root before any ansible code has been loaded
+    # otherwise config will have already been loaded and changing the environment will have no effect
+    monkeypatch.setenv('ANSIBLE_COLLECTIONS_PATHS', collection_root)
+
+    from ansible.utils.collection_loader import AnsibleCollectionLoader
+
+    for index in [idx for idx, obj in enumerate(sys.meta_path) if isinstance(obj, AnsibleCollectionLoader)]:
+        # replace any existing collection loaders that may exist
+        # since these were loaded during unit test collection
+        # they will not have the correct configuration
+        sys.meta_path[index] = AnsibleCollectionLoader()
+
+    # make sure the collection loader is installed
+    # this will be a no-op if the collection loader is already installed
+    # which will depend on whether or not any tests being run imported ansible.plugins.loader during unit test collection
+    from ansible.plugins.loader import _configure_collection_loader
+    _configure_collection_loader()  # currently redundant, the import above already calls this
+
+    from ansible_collections.my_namespace.my_collection.plugins.module_utils.my_util import question
+
+    original_trace_function = sys.gettrace()
+    trace_log = []
+
+    if original_trace_function:
+        # enable tracing while preserving the existing trace function (coverage)
+        def my_trace_function(frame, event, arg):
+            trace_log.append((frame.f_code.co_filename, frame.f_lineno, event))
+
+            # the original trace function expects to have itself set as the trace function
+            sys.settrace(original_trace_function)
+            # call the original trace function
+            original_trace_function(frame, event, arg)
+            # restore our trace function
+            sys.settrace(my_trace_function)
+
+            return my_trace_function
+    else:
+        # no existing trace function, so our trace function is much simpler
+        def my_trace_function(frame, event, arg):
+            trace_log.append((frame.f_code.co_filename, frame.f_lineno, event))
+
+            return my_trace_function
+
+    sys.settrace(my_trace_function)
+
+    try:
+        # run a minimal amount of code while the trace is running
+        # adding more code here, including use of a context manager, will add more to our trace
+        answer = question()
+    finally:
+        sys.settrace(original_trace_function)
+
+    # make sure relative imports work from collections code
+    # these require __package__ to be set correctly
+    import ansible_collections.my_namespace.my_collection.plugins.module_utils.my_other_util
+    import ansible_collections.my_namespace.my_collection.plugins.action.my_action
+
+    # verify that code loaded from a collection does not inherit __future__ statements from the collection loader
+    if sys.version_info[0] == 2:
+        # if the collection code inherits the division future feature from the collection loader this will fail
+        assert answer == 1
+    else:
+        assert answer == 1.5
+
+    # verify that the filename and line number reported by the trace is correct
+    # this makes sure that collection loading preserves file paths and line numbers
+    assert trace_log == expected_trace_log