From 3b00014428689ff9b4c5ef9cd2c3f99fcb1e5788 Mon Sep 17 00:00:00 2001
From: James Garner <james.garner@canonical.com>
Date: Fri, 11 Oct 2024 09:30:15 +1300
Subject: [PATCH] test: out of sheer superstition, revert last commit to rerun
 tests

This reverts commit 7f0f6caccf1c2fbbc25f161d81be5591befe869a.

Integration tests fail on the last commit (7f0f6...) -- consistently,
threee times.

They passed on the previous commit. What's going on?
---
 tests/validate/test_facades.py | 82 +++++++++++++++++++---------------
 1 file changed, 45 insertions(+), 37 deletions(-)

diff --git a/tests/validate/test_facades.py b/tests/validate/test_facades.py
index c5caef12..2b0ada18 100644
--- a/tests/validate/test_facades.py
+++ b/tests/validate/test_facades.py
@@ -4,7 +4,7 @@
 import importlib
 import re
 from pathlib import Path
-from typing import Dict, List, TypedDict, cast
+from typing import Dict, List, Set, Tuple, TypedDict, cast
 
 import pytest
 
@@ -23,52 +23,60 @@ def project_root(pytestconfig: pytest.Config) -> Path:
     return pytestconfig.rootpath
 
 
-def test_facade_version_matches_filename(project_root: Path) -> None:
-    ignore_names = dir(_definitions)
-    for file in (project_root / 'juju' / 'client').glob('_client[0-9]*.py'):
-        match = re.search('_client([0-9]+).py', file.name)
-        assert match
-        version = int(match.group(1))
-        module = importlib.import_module(f'juju.client.{file.stem}')
-        for cls_name in dir(module):
-            if cls_name.startswith('_') or cls_name in ignore_names:
-                continue
-            assert getattr(module, cls_name).version == version
-
-
-def test_class_name_matches_facade_name(project_root: Path) -> None:
-    ignore_names = dir(_definitions)
-    for file in (project_root / 'juju' / 'client').glob('_client[0-9]*.py'):
-        module = importlib.import_module(f'juju.client.{file.stem}')
-        for cls_name in dir(module):
-            if cls_name.startswith('_') or cls_name in ignore_names:
-                continue
-            assert getattr(module, cls_name).name == cls_name.removesuffix('Facade')
-
-
-def test_client_facades_matches_generated_code(project_root: Path) -> None:
+def test_client_facades(project_root: Path) -> None:
     client_facades = cast(ClientFacades, connection.client_facades)
-    expected_facades = make_client_facades_from_generated_code(project_root)
+    good_facades = make_client_facades_from_generated_code(project_root)
     assert {
         k: v['versions'] for k, v in client_facades.items()
     } == {
-        k: v['versions'] for k, v in expected_facades.items()
+        k: v['versions'] for k, v in good_facades.items()
     }
 
 
 def make_client_facades_from_generated_code(project_root: Path) -> ClientFacades:
     """Return a client_facades dictionary from generated code under project_root.
     """
-    ignore_names = dir(_definitions)
-    excluded_facades = connection.excluded_facades
-    facades: Dict[str, List[int]] = {}
+    files_by_version: List[Tuple[int, Path]] = []
+    # [(facade_version, Path), ...]
     for file in (project_root / 'juju' / 'client').glob('_client[0-9]*.py'):
+        files_by_version.append((_version_from_filename(file), file))
+    files_by_version.sort()
+
+    # _clientN.py files import * from _definitions
+    # so we will ignore any names from there
+    ignore = dir(_definitions)
+
+    facades_by_version: Dict[int, Set[str]] = {}
+    # {facade_version: {facade_name, ...}, ...}
+    for version, file in files_by_version:
         module = importlib.import_module(f'juju.client.{file.stem}')
-        for cls_name in dir(module):
-            if cls_name.startswith('_') or cls_name in ignore_names:
-                continue
-            cls = getattr(module, cls_name)
-            if cls.version in excluded_facades.get(cls.name, []):
+        facades = {
+            name.removesuffix("Facade")
+            for name in dir(module)
+            if not (name.startswith('_') or name in ignore)
+        }
+        facades_by_version[version] = facades
+
+    # client_facades in connection.py is sorted
+    # so we sort facade names before constructing it
+    first, *rest = facades_by_version.values()
+    sorted_facade_names: list[str] = sorted(first.union(*rest))
+
+    excluded_facades = connection.excluded_facades
+    client_facades: ClientFacades = {}
+    # {facade_name: {'versions': [1, 2, 3, ...]}, ...}
+    for name in sorted_facade_names:
+        versions: List[int] = []
+        for version, facades in facades_by_version.items():
+            if version in excluded_facades.get(name, []):
                 continue
-            facades.setdefault(cls.name, []).append(cls.version)
-    return {name: {'versions': sorted(facades[name])} for name in sorted(facades)}
+            if name in facades:
+                versions.append(version)
+        client_facades[name] = {'versions': versions}
+    return client_facades
+
+
+def _version_from_filename(path: Path) -> int:
+    match = re.search('_client([0-9]+).py', path.name)
+    assert match
+    return int(match.group(1))