Refactor check utility functions

Co-Authored: Antoine Lambert <antoine.lambert@inria.fr>

swh/loader/tests/__init__.py

@@ -280,52 +280,57 @@ def fetch_nar_extids_from_checksums(
     return extids
 
 
-def assert_task_and_visit_type_match(module_name: str) -> None:
-    """This checks the tasks declared in the ``module_name`` have their loader visit
+def assert_task_and_visit_type_match(tasks_module_name: str) -> None:
+    """This checks the tasks declared in the ``tasks_module_name`` have their loader visit
     type and their associated task name matching. If that's not the case, that poses
     issues when scheduling visits.
 
+    Args:
+        tasks_module_name: Fully qualified name of SWH module defining celery tasks
+
     Raises:
         AssertionError: when there is a discrepancy between a visit type of a loader and
           the task name
 
     """
-    from collections import defaultdict
     from importlib import import_module
 
-    import celery.app.task
-
-    mod = import_module(module_name)
+    mod = import_module(tasks_module_name)
     task_names = [x for x in dir(mod) if x.startswith("load_")]
     loaders = [x for x in dir(mod) if x.endswith("Loader")]
-    loaders_lower = [loader.lower() for loader in loaders]
-
-    matching_visit_types: Dict[str, bool] = defaultdict(bool)
-    for task_name in task_names:
-        taskobj = getattr(mod, task_name)
-        assert isinstance(taskobj, celery.app.task.Task)
-        loader_type = task_name.replace("load_", "").replace("_", "")
-
-        for loader_name_lower in loaders_lower:
-            if loader_type in loader_name_lower:
-                break
-        else:
-            raise AssertionError(f"No loader matching {loader_type} in {loaders_lower}")
-
-        for loader in loaders:
-            if loader_type not in loader.lower():
-                continue
-
-            loader_cls = getattr(mod, loader)
-            visit_type = loader_cls.visit_type
-
-            matching_visit_type = visit_type.replace("-", "") == loader_type
-            assert (
-                matching_visit_type is True
-            ), f"Visit type <{visit_type}> does not match task name <{task_name}>"
-            matching_visit_types[task_name] = matching_visit_type
-
-    # We should have as many task names as we have matching visits
-    assert len(task_names) == len(matching_visit_types.values()) and all(
-        matching_visit_types.values()
-    )
+
+    for loader in loaders:
+        loader_cls = getattr(mod, loader)
+        visit_type = loader_cls.visit_type
+        task_function_name = f"load_{visit_type.replace('-', '_')}"
+        assert task_function_name in task_names, (
+            f"task function {task_function_name} for visit type {visit_type } "
+            f"is missing in {tasks_module_name}"
+        )
+
+
+def assert_module_tasks_are_scheduler_ready(packages: Iterable) -> None:
+    """This iterates over a list of packages and check that "tasks" modules declare
+    tasks ready to be scheduled. If any discrepancy exist between a task and its loader
+    visit type, those will never get picked up by the scheduler, they are not scheduler
+    ready. This is an incorrect behavior that needs to be fixed immediately. This check
+    ensures that any improper configured task/loader will be reported so developer can
+    fix it.
+
+    Args:
+        packages: List of imported swh packages
+
+    Raises:
+        AssertionError: when there is any discrepancy between a loader's visit type and
+          its associated task name in the ``packages`` list.
+
+    """
+
+    import pkgutil
+
+    for package in packages:
+        for _, modname, _ in pkgutil.walk_packages(
+            package.__path__, prefix=f"{package.__name__}."
+        ):
+            if modname.endswith(".tasks"):
+                assert_task_and_visit_type_match(modname)

swh/loader/tests/test_tasks.py

@@ -3,39 +3,11 @@
 # License: GNU General Public License version 3, or any later version
 # See top-level LICENSE file for more information
 
-import pkgutil
-
-from swh.loader.tests import assert_task_and_visit_type_match
+from swh.loader.tests import assert_module_tasks_are_scheduler_ready
 
 
 def test_tasks_loader_visit_type_match_task_name():
     import swh.loader.core
     import swh.loader.package
 
-    modules_ok = []
-    modules_ko = []
-
-    modules_seen = set()
-    for package in [swh.loader.core, swh.loader.package]:
-        package = swh.loader
-        prefix = f"{package.__name__}."
-        for importer, modname, ispkg in pkgutil.walk_packages(
-            package.__path__, prefix=prefix
-        ):
-            if modname in modules_seen:
-                continue
-            modules_seen.add(modname)
-            # Don't look into test module nor in other non-core or non-package loaders
-            if "tasks" in modname and ("package" in modname or "core" in modname):
-                try:
-                    assert_task_and_visit_type_match(modname)
-                    modules_ok.append(modname)
-                except AssertionError as e:
-                    modules_ko.append((modname, e.args[0]))
-
-    # Raise an error with all problematic modules
-    if len(modules_ko) > 0:
-        error_message = "\n" + "\n".join(
-            f"{module}: {error}" for module, error in modules_ko
-        )
-        raise AssertionError(error_message)
+    assert_module_tasks_are_scheduler_ready([swh.loader.core, swh.loader.package])