revert(load_module): restore loose cache-hit; rewrite stub-based test as subprocess

CI on `platform_checks (ubuntu-latest, 3.12)` surfaced the
"Found many classes with name 'EchoBehaviour' / 'DefaultDialogues'"
regression in ``tests/test_act_storage.py``,
``tests/test_docs/test_build_aea_programmatically/test_programmatic_aea.py``,
and ``tests/test_connections/test_scaffold.py`` — the failure mode
that ``a0d759e7c`` (drop ``__file__`` equality from cache-hit check)
was written to fix. Commit ``96b1785c7`` had re-introduced it by
restoring the strict check.

Reverts ``96b1785c7``: ``aea.helpers.base.load_module`` reuses any
module already registered at the requested canonical dotted path,
regardless of which physical file the second call points at. This
is the cache-keep-first behaviour the rest of this PR depends on —
it lets open-autonomy's ``_MetaPayload.registry`` collapse the
short+long duplicate keys into a single class object.

``tests/test_cli/test_run.py::TestRunFailsWhenConnectionClassNotPresent::test_run``
was the one test that depended on the strict-cache behaviour: it
stubs ``http_client/connection.py`` with a class-less body and
expects the next ``aea run`` to read the stub from disk. Under
cache-keep-first the cached real ``http_client`` connection
(loaded by an earlier test in the same pytest session) is reused
and the stub on disk is ignored, so the documented "class not
found" error never fires and the agent boots.

Rewrites the test to invoke ``aea run`` via ``PexpectWrapper`` —
the pattern already used by the happy-path ``test_run[None]``
above. A subprocess starts with an empty ``sys.modules``, which is
exactly how production ``aea run`` runs, so the cache-vs-disk
question doesn't arise and the test exercises the same code path
an end user hits. Asserts the documented "Connection class
'HTTPClientConnection' not found" message and a non-zero exit
code.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: DIvyaNautiyal07

aea/helpers/base.py

@@ -127,37 +127,31 @@ def load_module(dotted_path: str, filepath: Path) -> types.ModuleType:
     :raises ValueError: if the filepath provided is not a module.  # noqa: DAR402
     :raises Exception: if the execution of the module raises exception.  # noqa: DAR402
     """
-    # If the dotted path is already in ``sys.modules`` AND the cached
-    # module's ``__file__`` resolves to the same path we were asked to
-    # load, reuse it. Reuse preserves class identity for callers that
-    # already hold a reference (e.g. via an earlier ``from ... import
-    # X``). When the requested ``filepath`` differs from the cached
-    # ``__file__`` (a caller replaced the source on disk, or two distinct
-    # copies of the same package live at different paths), re-execute
-    # so the caller sees the file they pointed us at. An explicit
-    # ``None`` entry is CPython's block-import sentinel: raise
+    # If the dotted path is already in ``sys.modules``, reuse it instead
+    # of re-executing the file. Re-execution creates a duplicate copy of
+    # every class defined in the source, breaking class identity for
+    # callers that already hold a reference (e.g. via an earlier
+    # ``from ... import X``). ``__file__`` is intentionally NOT compared
+    # — in test fixtures the same logical package may be reached
+    # through different physical files (vendor copy in a tmpdir vs the
+    # source tree) but both register the same dotted path and must
+    # resolve to the same module object across the whole process. An
+    # explicit ``None`` entry is CPython's block-import sentinel: raise
     # ``ImportError`` to match the standard import semantics rather
     # than silently overwriting it with a fresh exec.
     if dotted_path in sys.modules:
         existing = sys.modules[dotted_path]
         if existing is None:
             raise ImportError(f"import of {dotted_path!r} halted; None in sys.modules")
-        existing_file = getattr(existing, "__file__", None)
-        if existing_file is not None:
-            try:
-                same_file = Path(existing_file).resolve() == Path(filepath).resolve()
-            except OSError:
-                same_file = False
-            if same_file:
-                return existing
+        return existing
     spec = importlib.util.spec_from_file_location(dotted_path, str(filepath))
     module = importlib.util.module_from_spec(cast(ModuleSpec, spec))
     # Register before exec so a downstream `import` of the same dotted
-    # path during exec resolves to this module. On exec failure pop the
-    # half-built stub — if a stale entry (different ``__file__``) was
-    # present it is discarded rather than restored, since the caller
-    # was asking us to load a different file and we have no reason to
-    # leave the stale module visible.
+    # path during exec resolves to this module. We only reach this point
+    # when the key was absent (the cache-hit and block-import branches
+    # above covered the other two cases), so on exec failure we pop the
+    # half-built stub rather than restoring a prior entry that, by
+    # construction, did not exist.
     sys.modules[dotted_path] = module
     try:
         spec.loader.exec_module(module)  # type: ignore

tests/test_cli/test_run.py

@@ -1336,19 +1336,43 @@ def setup_class(cls):
         ).write_text("import packages.valory.protocols.http")
 
     def test_run(self):
-        """Run the test."""
+        """Run the test.
+
+        ``aea run`` is invoked as a subprocess via ``PexpectWrapper`` —
+        the same pattern as ``test_run[None]`` above. A subprocess
+        starts with an empty ``sys.modules`` (the production scenario),
+        so ``load_module`` reads the stubbed ``connection.py`` from
+        disk and surfaces the documented "Connection class … not
+        found" error, exactly as an end user would see it.
+        """
         expected_message = (
-            "Package loading error: An error occurred while loading connection {}: Connection class '{"
-            "}' not found.".format(self.connection_id, "HTTPClientConnection")
+            f"Package loading error: An error occurred while loading "
+            f"connection {self.connection_id}: Connection class "
+            f"'HTTPClientConnection' not found."
         )
-        with pytest.raises(ClickException, match=expected_message):
-            self.run_cli_command(
+        process = PexpectWrapper(  # nosec
+            [
+                sys.executable,
+                "-m",
+                "aea.cli",
                 "--skip-consistency-check",
                 "run",
                 "--connections",
                 self.connection_id,
-                cwd=self._get_cwd(),
-            )
+            ],
+            cwd=self._get_cwd(),
+            env=os.environ.copy(),
+            maxread=10000,
+            encoding="utf-8",
+            logfile=sys.stdout,
+        )
+        try:
+            process.expect(re.escape(expected_message), timeout=30)
+            process.wait_to_complete(10)
+            assert process.returncode not in (None, 0)
+        finally:
+            process.terminate()
+            process.wait_to_complete(10)
 
 
 class TestRunFailsWhenProtocolConfigFileNotFound:

tests/test_skills/test_base.py

@@ -511,16 +511,17 @@ def test_load_module_pops_sys_modules_on_exec_failure(tmp_path):
         sys.modules.pop(dotted_path, None)
 
 
-def test_load_module_reuses_cached_module_for_same_file(tmp_path):
-    """`load_module` reuses the cached module when the file matches.
-
-    Re-executing the same source would create duplicate class objects
-    and break class identity for callers that already hold a reference
-    (e.g. via an earlier ``from ... import X``). When the caller asks
-    for the same dotted path with a *different* physical file, the
-    cache is bypassed and the new file is executed — the caller has
-    explicitly pointed `load_module` at a different source and
-    expects to see it.
+def test_load_module_reuses_cached_module_for_same_dotted_path(tmp_path):
+    """`load_module` reuses any module already cached at the dotted path.
+
+    Whether the second call points at the same physical file or a
+    different one (e.g. a vendor copy in a tmpdir vs the source tree
+    in test fixtures), `load_module` returns the cached object — never
+    re-executes. Re-execution would create duplicate class objects
+    for callers that already hold references to the original
+    definitions (e.g. via an earlier ``from ... import X``). Mirrors
+    Python's own import semantics: once registered, the cached module
+    is canonical for that dotted path.
     """
     import sys
 
@@ -537,21 +538,22 @@ def test_load_module_reuses_cached_module_for_same_file(tmp_path):
         first = load_module(dotted_path, same_src)
         first_marker = first.Marker  # type: ignore[attr-defined]
 
-        # Same file → cache hit, identical module object.
+        # Same file → reuse.
         second = load_module(dotted_path, same_src)
         assert second is first, "load_module re-executed for the same file"
         assert second.Marker is first_marker  # type: ignore[attr-defined]
 
-        # Different file at the same dotted path → re-execute. The
-        # returned module reflects the new source: ``Other`` is
-        # defined, ``Marker`` is not.
+        # Different file at the same dotted path → still reuse the
+        # cached module. ``Marker`` survives; ``Other`` is NOT defined
+        # on the returned module because the second source was never
+        # executed.
         third = load_module(dotted_path, other_src)
-        assert third is not first, (
-            "load_module returned the stale cached module instead of "
-            "executing the new source"
+        assert third is first, (
+            "load_module re-executed when the cached dotted path was "
+            "called with a different physical file"
         )
-        assert hasattr(third, "Other")
-        assert not hasattr(third, "Marker")
+        assert hasattr(third, "Marker")
+        assert not hasattr(third, "Other")
     finally:
         sys.modules.pop(dotted_path, None)