feat(repository): integrate ignore filtering into selection pipeline + add unit tests
This commit introduces proper handling of the `ignore: true` flag in the
repository selection mechanism and adds comprehensive unit tests for both
`ignored.py` and `selected.py`.
- `get_selected_repos()` now filters ignored repositories in all implicit
selection modes:
• filter-only mode (string/category/tag)
• `--all` mode
• CWD-based selection
- Explicit identifiers (e.g. `pkgmgr install ignored-repo`) **bypass**
ignore filtering, so the user can still operate directly on ignored
repositories if they ask for them explicitly.
- Added `_maybe_filter_ignored()` helper to handle logic cleanly and allow
future extension (e.g. integrating a CLI flag `--include-ignored`).
Under `tests/unit/pkgmgr/core/repository`:
1. **test_ignored.py**
• Ensures `filter_ignored()` removes repos with `ignore: true`
• Ensures empty lists are handled correctly
2. **test_selected.py**
Comprehensive coverage of the selection logic:
• Explicit identifiers bypass ignore filtering
• Filter-only mode excludes ignored repos unless `include_ignored=True`
• `--all` mode excludes ignored repos unless explicitly overridden
• CWD-based detection filters ignored repos unless explicitly overridden
Before this change, ignored repositories still appeared in `pkgmgr list`,
`pkgmgr status`, and other commands using `get_selected_repos()`.
This was unintuitive and broke the expected semantics of the `ignore`
attribute.
The new logic ensures ignored repositories are truly invisible unless
explicitly requested.
https://chatgpt.com/share/69383b41-50a0-800f-a2b9-c680cd96d9e9
This commit is contained in:
@@ -8,6 +8,7 @@ import re
|
|||||||
from typing import Any, Dict, List, Sequence
|
from typing import Any, Dict, List, Sequence
|
||||||
|
|
||||||
from pkgmgr.core.repository.resolve import resolve_repos
|
from pkgmgr.core.repository.resolve import resolve_repos
|
||||||
|
from pkgmgr.core.repository.ignored import filter_ignored
|
||||||
|
|
||||||
Repository = Dict[str, Any]
|
Repository = Dict[str, Any]
|
||||||
|
|
||||||
@@ -88,7 +89,7 @@ def _apply_filters(
|
|||||||
if not _match_pattern(ident_str, string_pattern):
|
if not _match_pattern(ident_str, string_pattern):
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Category filter: nur echte Kategorien, KEINE Tags
|
# Category filter: only real categories, NOT tags
|
||||||
if category_patterns:
|
if category_patterns:
|
||||||
cats: List[str] = []
|
cats: List[str] = []
|
||||||
cats.extend(map(str, repo.get("category_files", [])))
|
cats.extend(map(str, repo.get("category_files", [])))
|
||||||
@@ -106,7 +107,7 @@ def _apply_filters(
|
|||||||
if not ok:
|
if not ok:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Tag filter: ausschließlich YAML-Tags
|
# Tag filter: YAML tags only
|
||||||
if tag_patterns:
|
if tag_patterns:
|
||||||
tags: List[str] = list(map(str, repo.get("tags", [])))
|
tags: List[str] = list(map(str, repo.get("tags", [])))
|
||||||
if not tags:
|
if not tags:
|
||||||
@@ -124,16 +125,38 @@ def _apply_filters(
|
|||||||
|
|
||||||
return filtered
|
return filtered
|
||||||
|
|
||||||
|
|
||||||
|
def _maybe_filter_ignored(args, repos: List[Repository]) -> List[Repository]:
|
||||||
|
"""
|
||||||
|
Apply ignore filtering unless the caller explicitly opted to include ignored
|
||||||
|
repositories (via args.include_ignored).
|
||||||
|
|
||||||
|
Note: this helper is used only for *implicit* selections (all / filters /
|
||||||
|
by-directory). For *explicit* identifiers we do NOT filter ignored repos,
|
||||||
|
so the user can still target them directly if desired.
|
||||||
|
"""
|
||||||
|
include_ignored: bool = bool(getattr(args, "include_ignored", False))
|
||||||
|
if include_ignored:
|
||||||
|
return repos
|
||||||
|
return filter_ignored(repos)
|
||||||
|
|
||||||
|
|
||||||
def get_selected_repos(args, all_repositories: List[Repository]) -> List[Repository]:
|
def get_selected_repos(args, all_repositories: List[Repository]) -> List[Repository]:
|
||||||
"""
|
"""
|
||||||
Compute the list of repositories selected by CLI arguments.
|
Compute the list of repositories selected by CLI arguments.
|
||||||
|
|
||||||
Modes:
|
Modes:
|
||||||
- If identifiers are given: select via resolve_repos() from all_repositories.
|
- If identifiers are given: select via resolve_repos() from all_repositories.
|
||||||
- Else if any of --category/--string/--tag is used: start from all_repositories
|
Ignored repositories are *not* filtered here, so explicit identifiers
|
||||||
and apply filters.
|
always win.
|
||||||
- Else if --all is set: select all_repositories.
|
- Else if any of --category/--string/--tag is used: start from
|
||||||
- Else: try to select the repository of the current working directory.
|
all_repositories, apply filters and then drop ignored repos.
|
||||||
|
- Else if --all is set: select all_repositories and then drop ignored repos.
|
||||||
|
- Else: try to select the repository of the current working directory
|
||||||
|
and then drop it if it is ignored.
|
||||||
|
|
||||||
|
The ignore filter can be bypassed by setting args.include_ignored = True
|
||||||
|
(e.g. via a CLI flag --include-ignored).
|
||||||
"""
|
"""
|
||||||
identifiers: List[str] = getattr(args, "identifiers", []) or []
|
identifiers: List[str] = getattr(args, "identifiers", []) or []
|
||||||
use_all: bool = bool(getattr(args, "all", False))
|
use_all: bool = bool(getattr(args, "all", False))
|
||||||
@@ -143,18 +166,25 @@ def get_selected_repos(args, all_repositories: List[Repository]) -> List[Reposit
|
|||||||
|
|
||||||
has_filters = bool(category_patterns or string_pattern or tag_patterns)
|
has_filters = bool(category_patterns or string_pattern or tag_patterns)
|
||||||
|
|
||||||
# 1) Explicit identifiers win
|
# 1) Explicit identifiers win and bypass ignore filtering
|
||||||
if identifiers:
|
if identifiers:
|
||||||
base = resolve_repos(identifiers, all_repositories)
|
base = resolve_repos(identifiers, all_repositories)
|
||||||
return _apply_filters(base, string_pattern, category_patterns, tag_patterns)
|
return _apply_filters(base, string_pattern, category_patterns, tag_patterns)
|
||||||
|
|
||||||
# 2) Filter-only mode: start from all repositories
|
# 2) Filter-only mode: start from all repositories
|
||||||
if has_filters:
|
if has_filters:
|
||||||
return _apply_filters(list(all_repositories), string_pattern, category_patterns, tag_patterns)
|
base = _apply_filters(
|
||||||
|
list(all_repositories),
|
||||||
|
string_pattern,
|
||||||
|
category_patterns,
|
||||||
|
tag_patterns,
|
||||||
|
)
|
||||||
|
return _maybe_filter_ignored(args, base)
|
||||||
|
|
||||||
# 3) --all (no filters): all repos
|
# 3) --all (no filters): all repos
|
||||||
if use_all:
|
if use_all:
|
||||||
return list(all_repositories)
|
base = list(all_repositories)
|
||||||
|
return _maybe_filter_ignored(args, base)
|
||||||
|
|
||||||
# 4) Fallback: try to select repository of current working directory
|
# 4) Fallback: try to select repository of current working directory
|
||||||
cwd = os.path.abspath(os.getcwd())
|
cwd = os.path.abspath(os.getcwd())
|
||||||
@@ -164,7 +194,7 @@ def get_selected_repos(args, all_repositories: List[Repository]) -> List[Reposit
|
|||||||
if os.path.abspath(str(repo.get("directory", ""))) == cwd
|
if os.path.abspath(str(repo.get("directory", ""))) == cwd
|
||||||
]
|
]
|
||||||
if by_dir:
|
if by_dir:
|
||||||
return by_dir
|
return _maybe_filter_ignored(args, by_dir)
|
||||||
|
|
||||||
# No specific match -> empty list
|
# No specific match -> empty list
|
||||||
return []
|
return []
|
||||||
|
|||||||
29
tests/unit/pkgmgr/core/repository/test_ignored.py
Normal file
29
tests/unit/pkgmgr/core/repository/test_ignored.py
Normal file
@@ -0,0 +1,29 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
# -*- coding: utf-8 -*-
|
||||||
|
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
from pkgmgr.core.repository.ignored import filter_ignored
|
||||||
|
|
||||||
|
|
||||||
|
class TestFilterIgnored(unittest.TestCase):
|
||||||
|
def test_filter_ignored_removes_repos_with_ignore_true(self) -> None:
|
||||||
|
repos = [
|
||||||
|
{"provider": "github.com", "account": "user", "repository": "a", "ignore": True},
|
||||||
|
{"provider": "github.com", "account": "user", "repository": "b", "ignore": False},
|
||||||
|
{"provider": "github.com", "account": "user", "repository": "c"},
|
||||||
|
]
|
||||||
|
|
||||||
|
result = filter_ignored(repos)
|
||||||
|
|
||||||
|
identifiers = {(r["provider"], r["account"], r["repository"]) for r in result}
|
||||||
|
self.assertNotIn(("github.com", "user", "a"), identifiers)
|
||||||
|
self.assertIn(("github.com", "user", "b"), identifiers)
|
||||||
|
self.assertIn(("github.com", "user", "c"), identifiers)
|
||||||
|
|
||||||
|
def test_filter_ignored_empty_list_returns_empty_list(self) -> None:
|
||||||
|
self.assertEqual(filter_ignored([]), [])
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
180
tests/unit/pkgmgr/core/repository/test_selected.py
Normal file
180
tests/unit/pkgmgr/core/repository/test_selected.py
Normal file
@@ -0,0 +1,180 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
# -*- coding: utf-8 -*-
|
||||||
|
|
||||||
|
import os
|
||||||
|
import unittest
|
||||||
|
from types import SimpleNamespace
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
from pkgmgr.core.repository.selected import get_selected_repos
|
||||||
|
|
||||||
|
|
||||||
|
def _repo(
|
||||||
|
provider: str,
|
||||||
|
account: str,
|
||||||
|
repository: str,
|
||||||
|
ignore: bool | None = None,
|
||||||
|
directory: str | None = None,
|
||||||
|
):
|
||||||
|
repo = {
|
||||||
|
"provider": provider,
|
||||||
|
"account": account,
|
||||||
|
"repository": repository,
|
||||||
|
}
|
||||||
|
if ignore is not None:
|
||||||
|
repo["ignore"] = ignore
|
||||||
|
if directory is not None:
|
||||||
|
repo["directory"] = directory
|
||||||
|
return repo
|
||||||
|
|
||||||
|
|
||||||
|
class TestGetSelectedRepos(unittest.TestCase):
|
||||||
|
def setUp(self) -> None:
|
||||||
|
self.repo_ignored = _repo(
|
||||||
|
"github.com",
|
||||||
|
"user",
|
||||||
|
"ignored-repo",
|
||||||
|
ignore=True,
|
||||||
|
directory="/repos/github.com/user/ignored-repo",
|
||||||
|
)
|
||||||
|
self.repo_visible = _repo(
|
||||||
|
"github.com",
|
||||||
|
"user",
|
||||||
|
"visible-repo",
|
||||||
|
ignore=False,
|
||||||
|
directory="/repos/github.com/user/visible-repo",
|
||||||
|
)
|
||||||
|
self.all_repos = [self.repo_ignored, self.repo_visible]
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# 1) Explizite Identifier – ignorierte Repos dürfen ausgewählt werden
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
def test_identifiers_bypass_ignore_filter(self) -> None:
|
||||||
|
args = SimpleNamespace(
|
||||||
|
identifiers=["ignored-repo"], # matches by repository name
|
||||||
|
all=False,
|
||||||
|
category=[],
|
||||||
|
string="",
|
||||||
|
tag=[],
|
||||||
|
include_ignored=False, # should be ignored for explicit identifiers
|
||||||
|
)
|
||||||
|
|
||||||
|
selected = get_selected_repos(args, self.all_repos)
|
||||||
|
|
||||||
|
self.assertEqual(len(selected), 1)
|
||||||
|
self.assertIs(selected[0], self.repo_ignored)
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# 2) Filter-only Modus – ignorierte Repos werden rausgefiltert
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
def test_filter_mode_excludes_ignored_by_default(self) -> None:
|
||||||
|
# string-Filter, der beide Repos matchen würde
|
||||||
|
args = SimpleNamespace(
|
||||||
|
identifiers=[],
|
||||||
|
all=False,
|
||||||
|
category=[],
|
||||||
|
string="repo", # substring in beiden Namen
|
||||||
|
tag=[],
|
||||||
|
include_ignored=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
selected = get_selected_repos(args, self.all_repos)
|
||||||
|
|
||||||
|
self.assertEqual(len(selected), 1)
|
||||||
|
self.assertIs(selected[0], self.repo_visible)
|
||||||
|
|
||||||
|
def test_filter_mode_can_include_ignored_when_flag_set(self) -> None:
|
||||||
|
args = SimpleNamespace(
|
||||||
|
identifiers=[],
|
||||||
|
all=False,
|
||||||
|
category=[],
|
||||||
|
string="repo",
|
||||||
|
tag=[],
|
||||||
|
include_ignored=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
selected = get_selected_repos(args, self.all_repos)
|
||||||
|
|
||||||
|
# Beide Repos sollten erscheinen, weil include_ignored=True
|
||||||
|
self.assertEqual({r["repository"] for r in selected}, {"ignored-repo", "visible-repo"})
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# 3) --all Modus – ignorierte Repos werden per Default entfernt
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
def test_all_mode_excludes_ignored_by_default(self) -> None:
|
||||||
|
args = SimpleNamespace(
|
||||||
|
identifiers=[],
|
||||||
|
all=True,
|
||||||
|
category=[],
|
||||||
|
string="",
|
||||||
|
tag=[],
|
||||||
|
include_ignored=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
selected = get_selected_repos(args, self.all_repos)
|
||||||
|
|
||||||
|
self.assertEqual(len(selected), 1)
|
||||||
|
self.assertIs(selected[0], self.repo_visible)
|
||||||
|
|
||||||
|
def test_all_mode_can_include_ignored_when_flag_set(self) -> None:
|
||||||
|
args = SimpleNamespace(
|
||||||
|
identifiers=[],
|
||||||
|
all=True,
|
||||||
|
category=[],
|
||||||
|
string="",
|
||||||
|
tag=[],
|
||||||
|
include_ignored=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
selected = get_selected_repos(args, self.all_repos)
|
||||||
|
|
||||||
|
self.assertEqual(len(selected), 2)
|
||||||
|
self.assertCountEqual(
|
||||||
|
[r["repository"] for r in selected],
|
||||||
|
["ignored-repo", "visible-repo"],
|
||||||
|
)
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# 4) CWD-Modus – Repo anhand des aktuellen Verzeichnisses auswählen
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
def test_cwd_selection_excludes_ignored_by_default(self) -> None:
|
||||||
|
# Wir lassen CWD auf das Verzeichnis des ignorierten Repos zeigen.
|
||||||
|
cwd = os.path.abspath(self.repo_ignored["directory"])
|
||||||
|
|
||||||
|
args = SimpleNamespace(
|
||||||
|
identifiers=[],
|
||||||
|
all=False,
|
||||||
|
category=[],
|
||||||
|
string="",
|
||||||
|
tag=[],
|
||||||
|
include_ignored=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch("os.getcwd", return_value=cwd):
|
||||||
|
selected = get_selected_repos(args, self.all_repos)
|
||||||
|
|
||||||
|
# Da das einzige Repo für dieses Verzeichnis ignoriert ist,
|
||||||
|
# sollte die Auswahl leer sein.
|
||||||
|
self.assertEqual(selected, [])
|
||||||
|
|
||||||
|
def test_cwd_selection_can_include_ignored_when_flag_set(self) -> None:
|
||||||
|
cwd = os.path.abspath(self.repo_ignored["directory"])
|
||||||
|
|
||||||
|
args = SimpleNamespace(
|
||||||
|
identifiers=[],
|
||||||
|
all=False,
|
||||||
|
category=[],
|
||||||
|
string="",
|
||||||
|
tag=[],
|
||||||
|
include_ignored=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch("os.getcwd", return_value=cwd):
|
||||||
|
selected = get_selected_repos(args, self.all_repos)
|
||||||
|
|
||||||
|
self.assertEqual(len(selected), 1)
|
||||||
|
self.assertIs(selected[0], self.repo_ignored)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
Reference in New Issue
Block a user