Skip to content

Commit 6e58d9e

Browse files
committed
fix(page_package): prefer the package found in the working dir
- add find_first_package_dir, which returns the first search path match - use it for package info lookup instead of searching all paths at once - a package present in working dir and an additional dir no longer raises - closes #135
1 parent 0d32bc2 commit 6e58d9e

2 files changed

Lines changed: 111 additions & 10 deletions

File tree

‎src/osw/controller/page_package.py‎

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,32 @@ def find_package_dir(
248248
return matching_dirs[0]
249249

250250

251+
def find_first_package_dir(
252+
package_or_script_name: str, search_paths: List[Path] = None
253+
) -> Optional[Path]:
254+
"""searches the search_paths in order and returns the first match
255+
256+
Unlike find_package_dir, which searches all paths at once and rejects a
257+
name that exists in more than one of them, this prefers the earlier path.
258+
Returns None if no search path holds the name. A path holding more than one
259+
match is ambiguous on its own, so it is skipped with a warning.
260+
"""
261+
if search_paths is None:
262+
search_paths = []
263+
for search_path in search_paths:
264+
try:
265+
return find_package_dir(package_or_script_name, [search_path])
266+
except FileNotFoundError:
267+
# expected: not every search path holds every package
268+
continue
269+
except ValueError as e:
270+
warn(
271+
f"Multiple elements {package_or_script_name} found in "
272+
f"{search_path}: {e}"
273+
)
274+
return None
275+
276+
251277
def get_listed_pages_from_package_info(package_info: Union[dict, Path]) -> List[str]:
252278
"""Takes in the output of read_package_info_file and returns a list of
253279
pages listed in the package"""
@@ -681,17 +707,29 @@ def recursive(
681707
else:
682708
search_paths = [params.creation_config.working_dir.parent]
683709
search_paths.extend(params.additional_package_dirs)
684-
try:
685-
package_dir = find_package_dir(package_to_process, search_paths)
686-
package_info = read_package_info_file(package_dir)
687-
new_listed_pages = get_listed_pages_from_package_info(package_info)
688-
required_packages = get_required_packages_from_package_info_file(
689-
package_info
710+
# Prefer the package in the working dir over the additional
711+
# dirs, instead of failing on a package present in several.
712+
package_dir = find_first_package_dir(package_to_process, search_paths)
713+
new_listed_pages = []
714+
required_packages = []
715+
if package_dir is None:
716+
warn(
717+
f"Package info for {package_to_process} not found in any "
718+
f"of the search paths: {search_paths}"
690719
)
691-
except Exception as e:
692-
warn(f"Error reading package info for {package_to_process}: {e}")
693-
new_listed_pages = []
694-
required_packages = []
720+
else:
721+
try:
722+
package_info = read_package_info_file(package_dir)
723+
new_listed_pages = get_listed_pages_from_package_info(
724+
package_info
725+
)
726+
required_packages = (
727+
get_required_packages_from_package_info_file(package_info)
728+
)
729+
except Exception as e:
730+
warn(
731+
f"Error reading package info for {package_to_process}: {e}"
732+
)
695733
# Check for redundant pages
696734
for pack_ in listed_pages.keys():
697735
new_redundant_pages = list(

‎tests/test_page_package_search.py‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
"""Unit tests for package directory lookup in osw.controller.page_package.
2+
3+
Regression guard for #135: a package present in both the working dir and an
4+
additional package dir must resolve to the working dir one, instead of raising
5+
because the name was found more than once.
6+
"""
7+
8+
import pytest
9+
10+
from osw.controller.page_package import find_first_package_dir, find_package_dir
11+
12+
13+
@pytest.fixture
14+
def two_dirs(tmp_path):
15+
"""A working dir and an additional dir, both holding 'MyPackage'."""
16+
work_dir = tmp_path / "work"
17+
extra_dir = tmp_path / "extra"
18+
(work_dir / "MyPackage").mkdir(parents=True)
19+
(extra_dir / "MyPackage").mkdir(parents=True)
20+
return work_dir, extra_dir
21+
22+
23+
def test_prefers_the_first_search_path(two_dirs):
24+
work_dir, extra_dir = two_dirs
25+
26+
found = find_first_package_dir("MyPackage", [work_dir, extra_dir])
27+
28+
assert found == work_dir / "MyPackage"
29+
30+
31+
def test_search_order_decides(two_dirs):
32+
"""The same two dirs in the other order resolve to the other package."""
33+
work_dir, extra_dir = two_dirs
34+
35+
found = find_first_package_dir("MyPackage", [extra_dir, work_dir])
36+
37+
assert found == extra_dir / "MyPackage"
38+
39+
40+
def test_falls_through_to_a_later_path(two_dirs, tmp_path):
41+
_, extra_dir = two_dirs
42+
empty_dir = tmp_path / "empty"
43+
empty_dir.mkdir()
44+
45+
found = find_first_package_dir("MyPackage", [empty_dir, extra_dir])
46+
47+
assert found == extra_dir / "MyPackage"
48+
49+
50+
def test_returns_none_when_nothing_matches(tmp_path):
51+
assert find_first_package_dir("MyPackage", [tmp_path]) is None
52+
53+
54+
def test_returns_none_for_empty_search_paths():
55+
assert find_first_package_dir("MyPackage", None) is None
56+
57+
58+
def test_find_package_dir_still_rejects_ambiguity(two_dirs):
59+
"""The all-at-once helper keeps its previous behaviour."""
60+
work_dir, extra_dir = two_dirs
61+
62+
with pytest.raises(ValueError):
63+
find_package_dir("MyPackage", [work_dir, extra_dir])

0 commit comments

Comments
 (0)