diff options
| author | Victor Pushkarev <corvinus.v@gmail.com> | 2026-09-01 19:38:49 +0300 |
|---|---|---|
| committer | gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com <gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com> | 2026-09-11 01:54:27 -0700 |
| commit | cc88be34d214418511c13f2284ecdb22245fb650 (patch) | |
| tree | 83e205c55a3f20a1af71c2831ed413ba66b59541 /tests | |
| parent | 16cfb53e0ab9da8f40740885e5b287a0d912446c (diff) | |
| download | git-repo-cc88be34d214418511c13f2284ecdb22245fb650.tar.gz | |
command: fix group selection across submanifests
When GetProjects walks all manifests without explicit groups, it
resolves the effective groups once from the selected manifest and reuses
them for projects from child manifests. This can ignore a child
manifest's own group selection and include projects that should be
excluded.
When no groups are specified, resolve them from each project's owning
manifest. Explicit groups still apply to all projects in the call, and
single-manifest behavior remains unchanged.
Add regression tests for per-manifest group selection, explicit
overrides, and name lookup across manifests.
Bug: 382086294
Change-Id: Id5c7e52410431b86b395f489cfd494ca05620975
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/624821
Reviewed-by: Mike Frysinger <vapier@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Victor Pushkarev <corvinus.v@gmail.com>
Tested-by: Victor Pushkarev <corvinus.v@gmail.com>
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_command.py | 147 |
1 files changed, 144 insertions, 3 deletions
diff --git a/tests/test_command.py b/tests/test_command.py index d7938c5f2..ef6f7a031 100644 --- a/tests/test_command.py +++ b/tests/test_command.py | |||
| @@ -14,6 +14,8 @@ | |||
| 14 | 14 | ||
| 15 | """Unittests for the command.py module.""" | 15 | """Unittests for the command.py module.""" |
| 16 | 16 | ||
| 17 | from typing import Iterable, List, Optional | ||
| 18 | |||
| 17 | import pytest | 19 | import pytest |
| 18 | 20 | ||
| 19 | from command import Command | 21 | from command import Command |
| @@ -33,6 +35,8 @@ class FakeProject: | |||
| 33 | ): | 35 | ): |
| 34 | self.name = name | 36 | self.name = name |
| 35 | self.relpath = relpath | 37 | self.relpath = relpath |
| 38 | self.worktree = f"/work/{relpath}" | ||
| 39 | self.manifest = None | ||
| 36 | self.gitdir = gitdir or f"/git/{relpath}" | 40 | self.gitdir = gitdir or f"/git/{relpath}" |
| 37 | self.sync_s = sync_s | 41 | self.sync_s = sync_s |
| 38 | self.Exists = True | 42 | self.Exists = True |
| @@ -51,11 +55,58 @@ class FakeProject: | |||
| 51 | class FakeManifest: | 55 | class FakeManifest: |
| 52 | """Minimal manifest double for Command.GetProjects tests.""" | 56 | """Minimal manifest double for Command.GetProjects tests.""" |
| 53 | 57 | ||
| 54 | def __init__(self, projects): | 58 | def __init__( |
| 55 | self.projects = projects | 59 | self, |
| 60 | projects: Iterable[FakeProject], | ||
| 61 | *, | ||
| 62 | all_projects: Optional[Iterable[FakeProject]] = None, | ||
| 63 | effective_groups: str = "default", | ||
| 64 | ): | ||
| 65 | self.projects = list(projects) | ||
| 66 | self.all_projects = ( | ||
| 67 | list(self.projects) if all_projects is None else list(all_projects) | ||
| 68 | ) | ||
| 69 | self._effective_groups = effective_groups | ||
| 70 | |||
| 71 | # all_projects may include projects owned by child manifests, | ||
| 72 | # so only set this manifest on its direct projects. | ||
| 73 | for project in self.projects: | ||
| 74 | self._set_project_manifest(project) | ||
| 75 | |||
| 76 | def _set_project_manifest(self, project: FakeProject) -> None: | ||
| 77 | project.manifest = self | ||
| 78 | for subproject in project.GetDerivedSubprojects(): | ||
| 79 | self._set_project_manifest(subproject) | ||
| 56 | 80 | ||
| 57 | def GetManifestGroupsStr(self): | 81 | def GetManifestGroupsStr(self): |
| 58 | return "default" | 82 | return self._effective_groups |
| 83 | |||
| 84 | def GetProjectsWithName( | ||
| 85 | self, name: str, all_manifests: bool = False | ||
| 86 | ) -> List[FakeProject]: | ||
| 87 | projects = self.all_projects if all_manifests else self.projects | ||
| 88 | return [project for project in projects if project.name == name] | ||
| 89 | |||
| 90 | |||
| 91 | class GroupMatchingFakeProject(FakeProject): | ||
| 92 | """Fake project with predictable group matches for GetProjects tests. | ||
| 93 | |||
| 94 | This lets the tests check which groups GetProjects uses without | ||
| 95 | reimplementing Project.MatchesGroups. | ||
| 96 | """ | ||
| 97 | |||
| 98 | def __init__( | ||
| 99 | self, | ||
| 100 | name: str, | ||
| 101 | relpath: str, | ||
| 102 | *, | ||
| 103 | matching_groups: Iterable[str], | ||
| 104 | ): | ||
| 105 | super().__init__(name, relpath) | ||
| 106 | self._matching_groups = set(matching_groups) | ||
| 107 | |||
| 108 | def MatchesGroups(self, groups: Iterable[str]) -> bool: | ||
| 109 | return bool(self._matching_groups.intersection(groups)) | ||
| 59 | 110 | ||
| 60 | 111 | ||
| 61 | def test_get_projects_keeps_derived_subprojects_for_repeated_repo(): | 112 | def test_get_projects_keeps_derived_subprojects_for_repeated_repo(): |
| @@ -117,3 +168,93 @@ def test_get_projects_submodule_override( | |||
| 117 | projects = cmd.GetProjects([], submodules_ok=submodules_ok) | 168 | projects = cmd.GetProjects([], submodules_ok=submodules_ok) |
| 118 | 169 | ||
| 119 | assert (submodule in projects) is includes_submodule | 170 | assert (submodule in projects) is includes_submodule |
| 171 | |||
| 172 | |||
| 173 | @pytest.mark.parametrize( | ||
| 174 | ("groups", "expected_relpaths"), | ||
| 175 | [ | ||
| 176 | (None, ["outer", "sub/child"]), | ||
| 177 | ("", ["outer", "sub/child"]), | ||
| 178 | ("override-group", ["sub/override"]), | ||
| 179 | ], | ||
| 180 | ids=("groups-omitted", "groups-empty", "explicit-override"), | ||
| 181 | ) | ||
| 182 | def test_get_projects_uses_groups_from_each_manifest_unless_overridden( | ||
| 183 | groups: Optional[str], | ||
| 184 | expected_relpaths: List[str], | ||
| 185 | ) -> None: | ||
| 186 | """Use each manifest's effective groups unless the caller overrides them.""" | ||
| 187 | outer_project = GroupMatchingFakeProject( | ||
| 188 | "outer", | ||
| 189 | "outer", | ||
| 190 | matching_groups={"outer-group"}, | ||
| 191 | ) | ||
| 192 | |||
| 193 | # Both child projects also match "outer". Reusing the outer manifest's | ||
| 194 | # groups would therefore select both child projects. | ||
| 195 | child_project = GroupMatchingFakeProject( | ||
| 196 | "child", | ||
| 197 | "sub/child", | ||
| 198 | matching_groups={"outer-group", "child-group"}, | ||
| 199 | ) | ||
| 200 | override_project = GroupMatchingFakeProject( | ||
| 201 | "override", | ||
| 202 | "sub/override", | ||
| 203 | matching_groups={"outer-group", "override-group"}, | ||
| 204 | ) | ||
| 205 | |||
| 206 | child_manifest = FakeManifest( | ||
| 207 | [child_project, override_project], | ||
| 208 | effective_groups="child-group", | ||
| 209 | ) | ||
| 210 | outer_manifest = FakeManifest( | ||
| 211 | [outer_project], | ||
| 212 | all_projects=[outer_project, *child_manifest.projects], | ||
| 213 | effective_groups="outer-group", | ||
| 214 | ) | ||
| 215 | cmd = Command(manifest=outer_manifest) | ||
| 216 | |||
| 217 | projects = cmd.GetProjects( | ||
| 218 | [], | ||
| 219 | manifest=outer_manifest, | ||
| 220 | groups=groups, | ||
| 221 | all_manifests=True, | ||
| 222 | ) | ||
| 223 | |||
| 224 | assert [project.relpath for project in projects] == expected_relpaths | ||
| 225 | |||
| 226 | |||
| 227 | def test_get_projects_by_name_uses_groups_from_each_manifest() -> None: | ||
| 228 | """Name matches use the groups from each project's owning manifest.""" | ||
| 229 | outer_project = GroupMatchingFakeProject( | ||
| 230 | "shared", | ||
| 231 | "outer/shared", | ||
| 232 | matching_groups={"outer-group"}, | ||
| 233 | ) | ||
| 234 | child_project = GroupMatchingFakeProject( | ||
| 235 | "shared", | ||
| 236 | "sub/shared", | ||
| 237 | matching_groups={"child-group"}, | ||
| 238 | ) | ||
| 239 | |||
| 240 | child_manifest = FakeManifest( | ||
| 241 | [child_project], | ||
| 242 | effective_groups="child-group", | ||
| 243 | ) | ||
| 244 | outer_manifest = FakeManifest( | ||
| 245 | [outer_project], | ||
| 246 | all_projects=[outer_project, *child_manifest.projects], | ||
| 247 | effective_groups="outer-group", | ||
| 248 | ) | ||
| 249 | cmd = Command(manifest=outer_manifest) | ||
| 250 | |||
| 251 | projects = cmd.GetProjects( | ||
| 252 | ["shared"], | ||
| 253 | manifest=outer_manifest, | ||
| 254 | all_manifests=True, | ||
| 255 | ) | ||
| 256 | |||
| 257 | assert [project.relpath for project in projects] == [ | ||
| 258 | "outer/shared", | ||
| 259 | "sub/shared", | ||
| 260 | ] | ||
