-
Notifications
You must be signed in to change notification settings - Fork 53
fix(git): import branches that conflict with the default #9462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stable
Are you sure you want to change the base?
Changes from all commits
41af319
3482995
04e239b
4332e48
a0009a6
f4859fd
9986232
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,7 @@ | ||
| import logging | ||
| from collections.abc import Iterator | ||
| from pathlib import Path | ||
| from unittest.mock import patch | ||
|
|
||
| import pytest | ||
| from git import Repo | ||
|
|
@@ -11,16 +14,110 @@ | |
| from tests.helpers.file_repo import MultipleStagesFileRepo | ||
| from tests.helpers.test_client import dummy_async_request | ||
|
|
||
| PREFECT_LOGGER_NAME = "infrahub.git.base" | ||
|
|
||
| async def test_has_conflicting_changes_no_false_positive( | ||
|
|
||
| @pytest.fixture | ||
| def patch_prefect_logger() -> Iterator[None]: | ||
| """Replace Prefect's `get_run_logger` with a stdlib logger so calls outside a flow context succeed.""" | ||
| with patch( | ||
| "infrahub.git.base.get_run_logger", | ||
| return_value=logging.getLogger(PREFECT_LOGGER_NAME), | ||
| ): | ||
| yield | ||
|
|
||
|
|
||
| def _build_source_with_conflicting_branches(source_dir: Path) -> None: | ||
| """Initialize a git source repo with `main` and `change1` whose tips edit the same lines.""" | ||
| source = Repo.init(source_dir, initial_branch="main") | ||
| with source.config_writer() as cfg: | ||
| cfg.set_value("user", "name", "Test") | ||
| cfg.set_value("user", "email", "test@test.local") | ||
| target = source_dir / "data.txt" | ||
| target.write_text("line 1\nline 2\nline 3\n", encoding="utf-8") | ||
| source.index.add(["data.txt"]) | ||
| base_commit = source.index.commit("base") | ||
|
|
||
| source.git.checkout("-b", "change1") | ||
| target.write_text("change1 version\nline 2\nline 3\n", encoding="utf-8") | ||
| source.index.add(["data.txt"]) | ||
| source.index.commit("change on change1") | ||
|
|
||
| source.git.checkout("main") | ||
| source.git.reset("--hard", base_commit.hexsha) | ||
| target.write_text("main version\nline 2\nline 3\n", encoding="utf-8") | ||
| source.index.add(["data.txt"]) | ||
| source.index.commit("change on main") | ||
|
|
||
|
|
||
| async def _build_repository_with_conflict( | ||
| tmp_path: Path, monkeypatch: pytest.MonkeyPatch, *, name: str = "conflicting-repo" | ||
| ) -> InfrahubRepository: | ||
| """Build an `InfrahubRepository` whose remote has `main` and a divergent `change1`. | ||
|
|
||
| Asserts the pre-condition that the two branches actually conflict so a helper drift | ||
| fails loudly instead of silently making downstream assertions meaningless. | ||
| """ | ||
| repos_dir = tmp_path / "repositories" | ||
| repos_dir.mkdir() | ||
| monkeypatch.setattr(registry, "_default_branch", "main") | ||
| monkeypatch.setattr(config.SETTINGS.git, "repositories_directory", str(repos_dir)) | ||
|
|
||
| source_dir = tmp_path / "source-repo" | ||
| source_dir.mkdir() | ||
| _build_source_with_conflicting_branches(source_dir) | ||
|
|
||
| repository = await InfrahubRepository.new( | ||
| id=UUIDT.new(), | ||
| name=name, | ||
| location=str(source_dir), | ||
| default_branch_name="main", | ||
| client=InfrahubClient(config=Config(requester=dummy_async_request)), | ||
| ) | ||
| assert repository.has_conflicting_changes(target_branch="main", source_branch="change1") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional comment: I am not sure about having hidden assertions in a test helper. Either this is testing the |
||
| return repository | ||
|
|
||
|
|
||
| async def test_create_branch_in_git_with_conflicting_remote_lands_at_remote_tip( | ||
| tmp_path: Path, | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| """has_conflicting_changes() should not report false positives when. | ||
| """A remote branch that conflicts with the default branch must still be imported locally. | ||
|
|
||
| The local branch should land at the remote tip so that downstream merge attempts can | ||
| surface the conflict at merge time, rather than aborting the entire import. | ||
| """ | ||
| repository = await _build_repository_with_conflict(tmp_path, monkeypatch) | ||
|
|
||
| remote_branches = repository.get_branches_from_remote() | ||
| expected_commit = remote_branches["change1"].commit | ||
|
|
||
| await repository.create_branch_in_git(branch_name="change1", branch_id=str(UUIDT.new())) | ||
|
|
||
| local_branches = repository.get_branches_from_local(include_worktree=False) | ||
| assert "change1" in local_branches | ||
| assert local_branches["change1"].commit == expected_commit | ||
|
|
||
| file content contains conflict marker characters like '======='. | ||
|
|
||
| async def test_validate_remote_branch_allows_conflicting_branch( | ||
| tmp_path: Path, | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| patch_prefect_logger: None, | ||
| ) -> None: | ||
| """validate_remote_branch must accept a branch that conflicts with the default branch. | ||
|
|
||
| Skipping the branch would prevent it from being imported. The conflict is surfaced at | ||
| merge time instead. | ||
| """ | ||
| repository = await _build_repository_with_conflict(tmp_path, monkeypatch) | ||
| assert repository.validate_remote_branch(branch_name="change1") is True | ||
|
|
||
|
|
||
| async def test_has_conflicting_changes_no_false_positive( | ||
| tmp_path: Path, | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| """has_conflicting_changes() must not flag a diff that only adds lines containing '======='.""" | ||
| repos_dir = tmp_path / "repositories" | ||
| repos_dir.mkdir() | ||
| monkeypatch.setattr(registry, "_default_branch", "main") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Importing a Git repository now succeeds even when one of its branches has a merge conflict against the default branch. The conflicting branch is imported as-is and a warning is logged; the conflict is reported when a user later attempts to merge the branch. |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More a thought than a feedback: in this situation,
Truemeans "Git is not able to even determine if there is a conflict". But it also means "All good, branch is clean" and "There are merge conflicts". If we want to take actions in the future, maybe discretizing these different states besides pushing logs could be useful.