From 122248ef9ca06a98bd21613f8fe133830adbd8b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20P=C3=A9rez?= Date: Sat, 7 Jun 2025 20:35:47 +0200 Subject: [PATCH 1/6] Add GitLab support for CHANGELOG.md --- pr_agent/git_providers/gitlab_provider.py | 44 +++- pr_agent/tools/pr_update_changelog.py | 10 +- tests/unittest/test_gitlab_provider.py | 147 ++++++++++++ tests/unittest/test_pr_update_changelog.py | 247 +++++++++++++++++++++ 4 files changed, 442 insertions(+), 6 deletions(-) create mode 100644 tests/unittest/test_gitlab_provider.py create mode 100644 tests/unittest/test_pr_update_changelog.py diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index df18c957..84f04cdc 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -1,8 +1,8 @@ import difflib import hashlib import re -from typing import Optional, Tuple -from urllib.parse import urlparse +from typing import Optional, Tuple, Any, Union +from urllib.parse import urlparse, parse_qs import gitlab import requests @@ -53,7 +53,7 @@ class GitLabProvider(GitProvider): def is_supported(self, capability: str) -> bool: if capability in ['get_issue_comments', 'create_inline_comment', 'publish_inline_comments', - 'publish_file_comments']: # gfm_markdown is supported in gitlab ! + 'publish_file_comments', 'gfm_markdown']: # gfm_markdown is supported in gitlab ! return False return True @@ -112,14 +112,50 @@ class GitLabProvider(GitProvider): get_logger().error(f"Could not get diff for merge request {self.id_mr}") raise DiffNotFoundError(f"Could not get diff for merge request {self.id_mr}") from e + def _ensure_string_content(self, content: Union[str, bytes]) -> str: + """Convert bytes content to UTF-8 string if needed.""" + if isinstance(content, bytes): + return content.decode('utf-8') + return content def get_pr_file_content(self, file_path: str, branch: str) -> str: try: - return self.gl.projects.get(self.id_project).files.get(file_path, branch).decode() + file_obj = self.gl.projects.get(self.id_project).files.get(file_path, branch) + content = file_obj.decode() + return self._ensure_string_content(content) except GitlabGetError: # In case of file creation the method returns GitlabGetError (404 file not found). # In this case we return an empty string for the diff. return '' + except Exception as e: + get_logger().warning(f"Error retrieving file {file_path} from branch {branch}: {e}") + return '' + + def create_or_update_pr_file(self, file_path: str, branch: str, contents="", message="") -> None: + """Create or update a file in the GitLab repository.""" + try: + project = self.gl.projects.get(self.id_project) + + if not message: + action = "Update" if contents else "Create" + message = f"{action} {file_path}" + + try: + existing_file = project.files.get(file_path, branch) + existing_file.content = contents + existing_file.save(branch=branch, commit_message=message) + get_logger().debug(f"Updated file {file_path} in branch {branch}") + except GitlabGetError: + project.files.create({ + 'file_path': file_path, + 'branch': branch, + 'content': contents, + 'commit_message': message + }) + get_logger().debug(f"Created file {file_path} in branch {branch}") + except Exception as e: + get_logger().exception(f"Failed to create or update file {file_path} in branch {branch}: {e}") + raise def get_diff_files(self) -> list[FilePatchInfo]: """ diff --git a/pr_agent/tools/pr_update_changelog.py b/pr_agent/tools/pr_update_changelog.py index 5b21785b..963cbe9e 100644 --- a/pr_agent/tools/pr_update_changelog.py +++ b/pr_agent/tools/pr_update_changelog.py @@ -58,7 +58,7 @@ class PRUpdateChangelog: 'config': dict(get_settings().config)} get_logger().debug("Relevant configs", artifacts=relevant_configs) - # currently only GitHub is supported for pushing changelog changes + # check if the git provider supports pushing changelog changes if get_settings().pr_update_changelog.push_changelog_changes and not hasattr( self.git_provider, "create_or_update_pr_file" ): @@ -128,6 +128,7 @@ class PRUpdateChangelog: existing_content = self.changelog_file else: existing_content = "" + if existing_content: new_file_content = answer + "\n\n" + self.changelog_file else: @@ -186,10 +187,15 @@ Example: self.changelog_file = self.git_provider.get_pr_file_content( "CHANGELOG.md", self.git_provider.get_pr_branch() ) + + if isinstance(self.changelog_file, bytes): + self.changelog_file = self.changelog_file.decode('utf-8') + changelog_file_lines = self.changelog_file.splitlines() changelog_file_lines = changelog_file_lines[:CHANGELOG_LINES] self.changelog_file_str = "\n".join(changelog_file_lines) - except Exception: + except Exception as e: + get_logger().warning(f"Error getting changelog file: {e}") self.changelog_file_str = "" self.changelog_file = "" diff --git a/tests/unittest/test_gitlab_provider.py b/tests/unittest/test_gitlab_provider.py new file mode 100644 index 00000000..d1889a35 --- /dev/null +++ b/tests/unittest/test_gitlab_provider.py @@ -0,0 +1,147 @@ +import pytest +from unittest.mock import MagicMock, patch + +from pr_agent.git_providers.gitlab_provider import GitLabProvider +from gitlab import Gitlab +from gitlab.v4.objects import Project, ProjectFile +from gitlab.exceptions import GitlabGetError + + +class TestGitLabProvider: + """Test suite for GitLab provider functionality.""" + + @pytest.fixture + def mock_gitlab_client(self): + client = MagicMock() + return client + + @pytest.fixture + def mock_project(self): + project = MagicMock() + return project + + @pytest.fixture + def gitlab_provider(self, mock_gitlab_client, mock_project): + with patch('pr_agent.git_providers.gitlab_provider.gitlab.Gitlab', return_value=mock_gitlab_client), \ + patch('pr_agent.git_providers.gitlab_provider.get_settings') as mock_settings: + + mock_settings.return_value.get.side_effect = lambda key, default=None: { + "GITLAB.URL": "https://gitlab.com", + "GITLAB.PERSONAL_ACCESS_TOKEN": "fake_token" + }.get(key, default) + + mock_gitlab_client.projects.get.return_value = mock_project + provider = GitLabProvider("https://gitlab.com/test/repo/-/merge_requests/1") + provider.gl = mock_gitlab_client + provider.id_project = "test/repo" + return provider + + def test_get_pr_file_content_success(self, gitlab_provider, mock_project): + mock_file = MagicMock(ProjectFile) + mock_file.decode.return_value = "# Changelog\n\n## v1.0.0\n- Initial release" + mock_project.files.get.return_value = mock_file + + content = gitlab_provider.get_pr_file_content("CHANGELOG.md", "main") + + assert content == "# Changelog\n\n## v1.0.0\n- Initial release" + mock_project.files.get.assert_called_once_with("CHANGELOG.md", "main") + mock_file.decode.assert_called_once() + + def test_get_pr_file_content_with_bytes(self, gitlab_provider, mock_project): + mock_file = MagicMock(ProjectFile) + mock_file.decode.return_value = b"# Changelog\n\n## v1.0.0\n- Initial release" + mock_project.files.get.return_value = mock_file + + content = gitlab_provider.get_pr_file_content("CHANGELOG.md", "main") + + assert content == "# Changelog\n\n## v1.0.0\n- Initial release" + mock_project.files.get.assert_called_once_with("CHANGELOG.md", "main") + + def test_get_pr_file_content_file_not_found(self, gitlab_provider, mock_project): + mock_project.files.get.side_effect = GitlabGetError("404 Not Found") + + content = gitlab_provider.get_pr_file_content("CHANGELOG.md", "main") + + assert content == "" + mock_project.files.get.assert_called_once_with("CHANGELOG.md", "main") + + def test_get_pr_file_content_other_exception(self, gitlab_provider, mock_project): + mock_project.files.get.side_effect = Exception("Network error") + + content = gitlab_provider.get_pr_file_content("CHANGELOG.md", "main") + + assert content == "" + + def test_create_or_update_pr_file_create_new(self, gitlab_provider, mock_project): + mock_project.files.get.side_effect = GitlabGetError("404 Not Found") + mock_file = MagicMock() + mock_project.files.create.return_value = mock_file + + new_content = "# Changelog\n\n## v1.1.0\n- New feature" + commit_message = "Add CHANGELOG.md" + + gitlab_provider.create_or_update_pr_file( + "CHANGELOG.md", "feature-branch", new_content, commit_message + ) + + mock_project.files.get.assert_called_once_with("CHANGELOG.md", "feature-branch") + mock_project.files.create.assert_called_once_with({ + 'file_path': 'CHANGELOG.md', + 'branch': 'feature-branch', + 'content': new_content, + 'commit_message': commit_message, + }) + + def test_create_or_update_pr_file_update_existing(self, gitlab_provider, mock_project): + mock_file = MagicMock(ProjectFile) + mock_file.decode.return_value = "# Old changelog content" + mock_project.files.get.return_value = mock_file + + new_content = "# New changelog content" + commit_message = "Update CHANGELOG.md" + + gitlab_provider.create_or_update_pr_file( + "CHANGELOG.md", "feature-branch", new_content, commit_message + ) + + mock_project.files.get.assert_called_once_with("CHANGELOG.md", "feature-branch") + mock_file.content = new_content + mock_file.save.assert_called_once_with(branch="feature-branch", commit_message=commit_message) + + def test_create_or_update_pr_file_update_exception(self, gitlab_provider, mock_project): + mock_project.files.get.side_effect = Exception("Network error") + + with pytest.raises(Exception): + gitlab_provider.create_or_update_pr_file( + "CHANGELOG.md", "feature-branch", "content", "message" + ) + + def test_has_create_or_update_pr_file_method(self, gitlab_provider): + assert hasattr(gitlab_provider, "create_or_update_pr_file") + assert callable(getattr(gitlab_provider, "create_or_update_pr_file")) + + def test_method_signature_compatibility(self, gitlab_provider): + import inspect + + sig = inspect.signature(gitlab_provider.create_or_update_pr_file) + params = list(sig.parameters.keys()) + + expected_params = ['file_path', 'branch', 'contents', 'message'] + assert params == expected_params + + @pytest.mark.parametrize("content,expected", [ + ("simple text", "simple text"), + (b"bytes content", "bytes content"), + ("", ""), + (b"", ""), + ("unicode: café", "unicode: café"), + (b"unicode: caf\xc3\xa9", "unicode: café"), + ]) + def test_content_encoding_handling(self, gitlab_provider, mock_project, content, expected): + mock_file = MagicMock(ProjectFile) + mock_file.decode.return_value = content + mock_project.files.get.return_value = mock_file + + result = gitlab_provider.get_pr_file_content("test.md", "main") + + assert result == expected \ No newline at end of file diff --git a/tests/unittest/test_pr_update_changelog.py b/tests/unittest/test_pr_update_changelog.py new file mode 100644 index 00000000..ce17e654 --- /dev/null +++ b/tests/unittest/test_pr_update_changelog.py @@ -0,0 +1,247 @@ +import pytest +from unittest.mock import MagicMock, patch, AsyncMock +from pr_agent.tools.pr_update_changelog import PRUpdateChangelog + + +class TestPRUpdateChangelog: + """Test suite for the PR Update Changelog functionality.""" + + @pytest.fixture + def mock_git_provider(self): + """Create a mock git provider.""" + provider = MagicMock() + provider.get_pr_branch.return_value = "feature-branch" + provider.get_pr_file_content.return_value = "" + provider.pr.title = "Test PR" + provider.get_pr_description.return_value = "Test description" + provider.get_commit_messages.return_value = "fix: test commit" + provider.get_languages.return_value = {"Python": 80, "JavaScript": 20} + provider.get_files.return_value = ["test.py", "test.js"] + return provider + + @pytest.fixture + def mock_ai_handler(self): + """Create a mock AI handler.""" + handler = MagicMock() + handler.chat_completion = AsyncMock(return_value=("Test changelog entry", "stop")) + return handler + + @pytest.fixture + def changelog_tool(self, mock_git_provider, mock_ai_handler): + """Create a PRUpdateChangelog instance with mocked dependencies.""" + with patch('pr_agent.tools.pr_update_changelog.get_git_provider', return_value=lambda url: mock_git_provider), \ + patch('pr_agent.tools.pr_update_changelog.get_main_pr_language', return_value="Python"), \ + patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings: + + # Configure mock settings + mock_settings.return_value.pr_update_changelog.push_changelog_changes = False + mock_settings.return_value.pr_update_changelog.extra_instructions = "" + mock_settings.return_value.pr_update_changelog_prompt.system = "System prompt" + mock_settings.return_value.pr_update_changelog_prompt.user = "User prompt" + mock_settings.return_value.config.temperature = 0.2 + + tool = PRUpdateChangelog("https://gitlab.com/test/repo/-/merge_requests/1", ai_handler=lambda: mock_ai_handler) + return tool + + def test_get_changelog_file_with_existing_content(self, changelog_tool, mock_git_provider): + """Test retrieving existing changelog content.""" + # Arrange + existing_content = "# Changelog\n\n## v1.0.0\n- Initial release\n- Bug fixes" + mock_git_provider.get_pr_file_content.return_value = existing_content + + # Act + changelog_tool._get_changelog_file() + + # Assert + assert changelog_tool.changelog_file == existing_content + assert "# Changelog" in changelog_tool.changelog_file_str + + def test_get_changelog_file_with_no_existing_content(self, changelog_tool, mock_git_provider): + """Test handling when no changelog file exists.""" + # Arrange + mock_git_provider.get_pr_file_content.return_value = "" + + # Act + changelog_tool._get_changelog_file() + + # Assert + assert changelog_tool.changelog_file == "" + assert "Example:" in changelog_tool.changelog_file_str # Default template + + def test_get_changelog_file_with_bytes_content(self, changelog_tool, mock_git_provider): + """Test handling when git provider returns bytes instead of string.""" + # Arrange + content_bytes = b"# Changelog\n\n## v1.0.0\n- Initial release" + mock_git_provider.get_pr_file_content.return_value = content_bytes + + # Act + changelog_tool._get_changelog_file() + + # Assert + assert isinstance(changelog_tool.changelog_file, str) + assert changelog_tool.changelog_file == "# Changelog\n\n## v1.0.0\n- Initial release" + + def test_get_changelog_file_with_exception(self, changelog_tool, mock_git_provider): + """Test handling exceptions during file retrieval.""" + # Arrange + mock_git_provider.get_pr_file_content.side_effect = Exception("Network error") + + # Act + changelog_tool._get_changelog_file() + + # Assert + assert changelog_tool.changelog_file == "" + assert changelog_tool.changelog_file_str != "" # Should have default content + + def test_prepare_changelog_update_with_existing_content(self, changelog_tool): + """Test preparing changelog update when existing content exists.""" + # Arrange + changelog_tool.prediction = "## v1.1.0\n- New feature\n- Bug fix" + changelog_tool.changelog_file = "# Changelog\n\n## v1.0.0\n- Initial release" + changelog_tool.commit_changelog = True + + # Act + new_content, answer = changelog_tool._prepare_changelog_update() + + # Assert + assert new_content.startswith("## v1.1.0\n- New feature\n- Bug fix\n\n") + assert "# Changelog\n\n## v1.0.0\n- Initial release" in new_content + assert answer == "## v1.1.0\n- New feature\n- Bug fix" + + def test_prepare_changelog_update_without_existing_content(self, changelog_tool): + """Test preparing changelog update when no existing content.""" + # Arrange + changelog_tool.prediction = "## v1.0.0\n- Initial release" + changelog_tool.changelog_file = "" + changelog_tool.commit_changelog = True + + # Act + new_content, answer = changelog_tool._prepare_changelog_update() + + # Assert + assert new_content == "## v1.0.0\n- Initial release" + assert answer == "## v1.0.0\n- Initial release" + + def test_prepare_changelog_update_no_commit(self, changelog_tool): + """Test preparing changelog update when not committing.""" + # Arrange + changelog_tool.prediction = "## v1.1.0\n- New feature" + changelog_tool.changelog_file = "" + changelog_tool.commit_changelog = False + + # Act + new_content, answer = changelog_tool._prepare_changelog_update() + + # Assert + assert new_content == "## v1.1.0\n- New feature" + assert "to commit the new content" in answer + + @pytest.mark.asyncio + async def test_run_without_push_support(self, changelog_tool, mock_git_provider): + """Test running changelog update when git provider doesn't support pushing.""" + # Arrange + delattr(mock_git_provider, 'create_or_update_pr_file') # Remove the method + changelog_tool.commit_changelog = True + + with patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings: + mock_settings.return_value.pr_update_changelog.push_changelog_changes = True + mock_settings.return_value.config.publish_output = True + + # Act + await changelog_tool.run() + + # Assert + mock_git_provider.publish_comment.assert_called_once() + assert "not currently supported" in str(mock_git_provider.publish_comment.call_args) + + @pytest.mark.asyncio + async def test_run_with_push_support(self, changelog_tool, mock_git_provider): + """Test running changelog update when git provider supports pushing.""" + # Arrange + mock_git_provider.create_or_update_pr_file = MagicMock() + changelog_tool.commit_changelog = True + changelog_tool.prediction = "## v1.1.0\n- New feature" + + with patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings, \ + patch('pr_agent.tools.pr_update_changelog.retry_with_fallback_models') as mock_retry, \ + patch('pr_agent.tools.pr_update_changelog.sleep'): + + mock_settings.return_value.pr_update_changelog.push_changelog_changes = True + mock_settings.return_value.pr_update_changelog.get.return_value = True + mock_settings.return_value.config.publish_output = True + mock_settings.return_value.config.git_provider = "gitlab" + mock_retry.return_value = None + + # Act + await changelog_tool.run() + + # Assert + mock_git_provider.create_or_update_pr_file.assert_called_once() + call_args = mock_git_provider.create_or_update_pr_file.call_args + assert call_args[1]['file_path'] == 'CHANGELOG.md' + assert call_args[1]['branch'] == 'feature-branch' + + def test_push_changelog_update(self, changelog_tool, mock_git_provider): + """Test the push changelog update functionality.""" + # Arrange + mock_git_provider.create_or_update_pr_file = MagicMock() + mock_git_provider.get_pr_branch.return_value = "feature-branch" + new_content = "# Updated changelog content" + answer = "Changes made" + + with patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings, \ + patch('pr_agent.tools.pr_update_changelog.sleep'): + + mock_settings.return_value.pr_update_changelog.get.return_value = True + + # Act + changelog_tool._push_changelog_update(new_content, answer) + + # Assert + mock_git_provider.create_or_update_pr_file.assert_called_once_with( + file_path="CHANGELOG.md", + branch="feature-branch", + contents=new_content, + message="[skip ci] Update CHANGELOG.md" + ) + + def test_gitlab_provider_method_detection(self, changelog_tool, mock_git_provider): + """Test that the tool correctly detects GitLab provider method availability.""" + # Arrange + mock_git_provider.create_or_update_pr_file = MagicMock() + + # Act & Assert + assert hasattr(mock_git_provider, "create_or_update_pr_file") + + @pytest.mark.parametrize("existing_content,new_entry,expected_order", [ + ( + "# Changelog\n\n## v1.0.0\n- Old feature", + "## v1.1.0\n- New feature", + ["v1.1.0", "v1.0.0"] + ), + ( + "", + "## v1.0.0\n- Initial release", + ["v1.0.0"] + ), + ( + "Some existing content", + "## v1.0.0\n- New entry", + ["v1.0.0", "Some existing content"] + ), + ]) + def test_changelog_order_preservation(self, changelog_tool, existing_content, new_entry, expected_order): + """Test that changelog entries are properly ordered (newest first).""" + # Arrange + changelog_tool.prediction = new_entry + changelog_tool.changelog_file = existing_content + changelog_tool.commit_changelog = True + + # Act + new_content, _ = changelog_tool._prepare_changelog_update() + + # Assert + for i, expected in enumerate(expected_order[:-1]): + current_pos = new_content.find(expected) + next_pos = new_content.find(expected_order[i + 1]) + assert current_pos < next_pos, f"Expected {expected} to come before {expected_order[i + 1]}" \ No newline at end of file From 04cb8af65d29f505c447910dd23be1e91ca19b71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20P=C3=A9rez?= Date: Sat, 7 Jun 2025 21:02:56 +0200 Subject: [PATCH 2/6] Fixed comment 1 --- pr_agent/git_providers/gitlab_provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index 84f04cdc..670d2c4c 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -53,7 +53,7 @@ class GitLabProvider(GitProvider): def is_supported(self, capability: str) -> bool: if capability in ['get_issue_comments', 'create_inline_comment', 'publish_inline_comments', - 'publish_file_comments', 'gfm_markdown']: # gfm_markdown is supported in gitlab ! + 'publish_file_comments']: # gfm_markdown is supported in gitlab ! return False return True From a550c174664ed50c405f70d2521df5b299780a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20P=C3=A9rez?= Date: Sat, 7 Jun 2025 21:06:12 +0200 Subject: [PATCH 3/6] Fixed comment 2 --- pr_agent/git_providers/gitlab_provider.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index 670d2c4c..bcd6616b 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -112,8 +112,10 @@ class GitLabProvider(GitProvider): get_logger().error(f"Could not get diff for merge request {self.id_mr}") raise DiffNotFoundError(f"Could not get diff for merge request {self.id_mr}") from e - def _ensure_string_content(self, content: Union[str, bytes]) -> str: + def _ensure_string_content(self, content: Union[str, bytes, None]) -> str: """Convert bytes content to UTF-8 string if needed.""" + if content is None: + return "" if isinstance(content, bytes): return content.decode('utf-8') return content From fe9afb826d267900800acc0dcddb6ebcd22b48e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20P=C3=A9rez?= Date: Sat, 7 Jun 2025 21:09:38 +0200 Subject: [PATCH 4/6] Fixed comment 3 --- pr_agent/git_providers/gitlab_provider.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index bcd6616b..8368984b 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -6,7 +6,7 @@ from urllib.parse import urlparse, parse_qs import gitlab import requests -from gitlab import GitlabGetError +from gitlab import GitlabGetError, GitlabAuthenticationError, GitlabCreateError, GitlabUpdateError from pr_agent.algo.types import EDIT_TYPE, FilePatchInfo @@ -155,8 +155,14 @@ class GitLabProvider(GitProvider): 'commit_message': message }) get_logger().debug(f"Created file {file_path} in branch {branch}") + except GitlabAuthenticationError as e: + get_logger().error(f"Authentication failed while creating/updating file {file_path} in branch {branch}: {e}") + raise + except (GitlabCreateError, GitlabUpdateError) as e: + get_logger().error(f"Permission denied or validation error for file {file_path} in branch {branch}: {e}") + raise except Exception as e: - get_logger().exception(f"Failed to create or update file {file_path} in branch {branch}: {e}") + get_logger().exception(f"Unexpected error creating/updating file {file_path} in branch {branch}: {e}") raise def get_diff_files(self) -> list[FilePatchInfo]: From 3904743e854b2553e85831f07c2878ccff80a4fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20P=C3=A9rez?= Date: Sat, 7 Jun 2025 21:13:16 +0200 Subject: [PATCH 5/6] Fixed comment 4 --- pr_agent/tools/pr_update_changelog.py | 1 + tests/unittest/test_pr_update_changelog.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pr_agent/tools/pr_update_changelog.py b/pr_agent/tools/pr_update_changelog.py index 963cbe9e..e6c1788b 100644 --- a/pr_agent/tools/pr_update_changelog.py +++ b/pr_agent/tools/pr_update_changelog.py @@ -198,6 +198,7 @@ Example: get_logger().warning(f"Error getting changelog file: {e}") self.changelog_file_str = "" self.changelog_file = "" + return if not self.changelog_file_str: self.changelog_file_str = self._get_default_changelog() diff --git a/tests/unittest/test_pr_update_changelog.py b/tests/unittest/test_pr_update_changelog.py index ce17e654..e3443896 100644 --- a/tests/unittest/test_pr_update_changelog.py +++ b/tests/unittest/test_pr_update_changelog.py @@ -91,7 +91,7 @@ class TestPRUpdateChangelog: # Assert assert changelog_tool.changelog_file == "" - assert changelog_tool.changelog_file_str != "" # Should have default content + assert changelog_tool.changelog_file_str == "" # Exception should result in empty string, no default template def test_prepare_changelog_update_with_existing_content(self, changelog_tool): """Test preparing changelog update when existing content exists.""" From d812b7ed7e9a37c37fb9178fdaac4981396e439f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20P=C3=A9rez?= Date: Wed, 11 Jun 2025 11:26:49 +0200 Subject: [PATCH 6/6] Fixed comment 5 --- pr_agent/git_providers/gitlab_provider.py | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index 8368984b..8cfe05f6 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -11,6 +11,7 @@ from gitlab import GitlabGetError, GitlabAuthenticationError, GitlabCreateError, from pr_agent.algo.types import EDIT_TYPE, FilePatchInfo from ..algo.file_filter import filter_ignored +from ..algo.git_patch_processing import decode_if_bytes from ..algo.language_handler import is_valid_file from ..algo.utils import (clip_tokens, find_line_number_of_relevant_line_in_file, @@ -112,19 +113,11 @@ class GitLabProvider(GitProvider): get_logger().error(f"Could not get diff for merge request {self.id_mr}") raise DiffNotFoundError(f"Could not get diff for merge request {self.id_mr}") from e - def _ensure_string_content(self, content: Union[str, bytes, None]) -> str: - """Convert bytes content to UTF-8 string if needed.""" - if content is None: - return "" - if isinstance(content, bytes): - return content.decode('utf-8') - return content - def get_pr_file_content(self, file_path: str, branch: str) -> str: try: file_obj = self.gl.projects.get(self.id_project).files.get(file_path, branch) content = file_obj.decode() - return self._ensure_string_content(content) + return decode_if_bytes(content) except GitlabGetError: # In case of file creation the method returns GitlabGetError (404 file not found). # In this case we return an empty string for the diff. @@ -211,14 +204,9 @@ class GitLabProvider(GitProvider): original_file_content_str = '' new_file_content_str = '' - try: - if isinstance(original_file_content_str, bytes): - original_file_content_str = bytes.decode(original_file_content_str, 'utf-8') - if isinstance(new_file_content_str, bytes): - new_file_content_str = bytes.decode(new_file_content_str, 'utf-8') - except UnicodeDecodeError: - get_logger().warning( - f"Cannot decode file {diff['old_path']} or {diff['new_path']} in merge request {self.id_mr}") + # Ensure content is properly decoded + original_file_content_str = decode_if_bytes(original_file_content_str) + new_file_content_str = decode_if_bytes(new_file_content_str) edit_type = EDIT_TYPE.MODIFIED if diff['new_file']: