fix(page_package): prefer the package found in the working dir

src/osw/controller/page_package.py

@@ -248,6 +248,32 @@ def find_package_dir(
         return matching_dirs[0]
 
 
+def find_first_package_dir(
+    package_or_script_name: str, search_paths: List[Path] = None
+) -> Optional[Path]:
+    """searches the search_paths in order and returns the first match
+
+    Unlike find_package_dir, which searches all paths at once and rejects a
+    name that exists in more than one of them, this prefers the earlier path.
+    Returns None if no search path holds the name. A path holding more than one
+    match is ambiguous on its own, so it is skipped with a warning.
+    """
+    if search_paths is None:
+        search_paths = []
+    for search_path in search_paths:
+        try:
+            return find_package_dir(package_or_script_name, [search_path])
+        except FileNotFoundError:
+            # expected: not every search path holds every package
+            continue
+        except ValueError as e:
+            warn(
+                f"Multiple elements {package_or_script_name} found in "
+                f"{search_path}: {e}"
+            )
+    return None
+
+
 def get_listed_pages_from_package_info(package_info: Union[dict, Path]) -> List[str]:
     """Takes in the output of read_package_info_file and returns a list of
     pages listed in the package"""
@@ -663,35 +689,58 @@ def recursive(
             if params.read_listed_pages_from_script:
                 search_paths = [params.script_dir]
                 search_paths.extend(params.additional_script_dirs)
-                try:
-                    script_path = find_package_dir(
-                        f"{package_to_process}.py", search_paths
-                    )
-                    package_script = read_package_script_file(script_path)
-                    new_listed_pages = get_listed_pages_from_package_script(
-                        package_script
-                    )
-                    required_packages = get_required_packages_from_package_script(
-                        package_script
+                # Prefer the script in script_dir over the additional dirs,
+                # instead of failing on a script present in several.
+                script_path = find_first_package_dir(
+                    f"{package_to_process}.py", search_paths
+                )
+                new_listed_pages = []
+                required_packages = []
+                if script_path is None:
+                    warn(
+                        f"Package script for {package_to_process} not found in any "
+                        f"of the search paths: {search_paths}"
                     )
-                except Exception as e:
-                    warn(f"Error reading package script for {package_to_process}: {e}")
-                    new_listed_pages = []
-                    required_packages = []
+                else:
+                    try:
+                        package_script = read_package_script_file(script_path)
+                        new_listed_pages = get_listed_pages_from_package_script(
+                            package_script
+                        )
+                        required_packages = get_required_packages_from_package_script(
+                            package_script
+                        )
+                    except Exception as e:
+                        warn(
+                            f"Error reading package script for "
+                            f"{package_to_process}: {e}"
+                        )
             else:
                 search_paths = [params.creation_config.working_dir.parent]
                 search_paths.extend(params.additional_package_dirs)
-                try:
-                    package_dir = find_package_dir(package_to_process, search_paths)
-                    package_info = read_package_info_file(package_dir)
-                    new_listed_pages = get_listed_pages_from_package_info(package_info)
-                    required_packages = get_required_packages_from_package_info_file(
-                        package_info
+                # Prefer the package in the working dir over the additional
+                # dirs, instead of failing on a package present in several.
+                package_dir = find_first_package_dir(package_to_process, search_paths)
+                new_listed_pages = []
+                required_packages = []
+                if package_dir is None:
+                    warn(
+                        f"Package info for {package_to_process} not found in any "
+                        f"of the search paths: {search_paths}"
                     )
-                except Exception as e:
-                    warn(f"Error reading package info for {package_to_process}: {e}")
-                    new_listed_pages = []
-                    required_packages = []
+                else:
+                    try:
+                        package_info = read_package_info_file(package_dir)
+                        new_listed_pages = get_listed_pages_from_package_info(
+                            package_info
+                        )
+                        required_packages = (
+                            get_required_packages_from_package_info_file(package_info)
+                        )
+                    except Exception as e:
+                        warn(
+                            f"Error reading package info for {package_to_process}: {e}"
+                        )
             # Check for redundant pages
             for pack_ in listed_pages.keys():
                 new_redundant_pages = list(

tests/test_page_package_search.py

@@ -0,0 +1,94 @@
+"""Unit tests for package directory lookup in osw.controller.page_package.
+
+Regression guard for #135: a package present in both the working dir and an
+additional package dir must resolve to the working dir one, instead of raising
+because the name was found more than once.
+"""
+
+import pytest
+
+from osw.controller.page_package import find_first_package_dir, find_package_dir
+
+
+@pytest.fixture
+def two_dirs(tmp_path):
+    """A working dir and an additional dir, both holding 'MyPackage'."""
+    work_dir = tmp_path / "work"
+    extra_dir = tmp_path / "extra"
+    (work_dir / "MyPackage").mkdir(parents=True)
+    (extra_dir / "MyPackage").mkdir(parents=True)
+    return work_dir, extra_dir
+
+
+def test_prefers_the_first_search_path(two_dirs):
+    work_dir, extra_dir = two_dirs
+
+    found = find_first_package_dir("MyPackage", [work_dir, extra_dir])
+
+    assert found == work_dir / "MyPackage"
+
+
+def test_search_order_decides(two_dirs):
+    """The same two dirs in the other order resolve to the other package."""
+    work_dir, extra_dir = two_dirs
+
+    found = find_first_package_dir("MyPackage", [extra_dir, work_dir])
+
+    assert found == extra_dir / "MyPackage"
+
+
+def test_falls_through_to_a_later_path(two_dirs, tmp_path):
+    _, extra_dir = two_dirs
+    empty_dir = tmp_path / "empty"
+    empty_dir.mkdir()
+
+    found = find_first_package_dir("MyPackage", [empty_dir, extra_dir])
+
+    assert found == extra_dir / "MyPackage"
+
+
+def test_returns_none_when_nothing_matches(tmp_path):
+    assert find_first_package_dir("MyPackage", [tmp_path]) is None
+
+
+def test_returns_none_for_empty_search_paths():
+    assert find_first_package_dir("MyPackage", None) is None
+
+
+def test_find_package_dir_still_rejects_ambiguity(two_dirs):
+    """The all-at-once helper keeps its previous behaviour."""
+    work_dir, extra_dir = two_dirs
+
+    with pytest.raises(ValueError):
+        find_package_dir("MyPackage", [work_dir, extra_dir])
+
+
+@pytest.fixture
+def two_script_dirs(tmp_path):
+    """A script dir and an additional dir, both holding 'MyPackage.py'."""
+    script_dir = tmp_path / "scripts"
+    extra_dir = tmp_path / "extra_scripts"
+    script_dir.mkdir()
+    extra_dir.mkdir()
+    (script_dir / "MyPackage.py").write_text("# working copy")
+    (extra_dir / "MyPackage.py").write_text("# dependency copy")
+    return script_dir, extra_dir
+
+
+def test_prefers_the_first_search_path_for_scripts(two_script_dirs):
+    """The script lookup takes the same route, so files must resolve too."""
+    script_dir, extra_dir = two_script_dirs
+
+    found = find_first_package_dir("MyPackage.py", [script_dir, extra_dir])
+
+    assert found == script_dir / "MyPackage.py"
+
+
+def test_falls_through_to_a_later_script_dir(two_script_dirs, tmp_path):
+    _, extra_dir = two_script_dirs
+    empty_dir = tmp_path / "empty_scripts"
+    empty_dir.mkdir()
+
+    found = find_first_package_dir("MyPackage.py", [empty_dir, extra_dir])
+
+    assert found == extra_dir / "MyPackage.py"