Merge pull request #1856 from cesdperez/feat/gitlab-changelogmd-support

Add GitLab support for CHANGELOG.md

pr_agent/git_providers/gitlab_provider.py

@@ -1,16 +1,17 @@
 import difflib
 import hashlib
 import re
-from typing import Optional, Tuple
+from typing import Optional, Tuple, Any, Union
-from urllib.parse import urlparse
+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
 
 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,14 +113,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 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 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.
             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 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"Unexpected error creating/updating file {file_path} in branch {branch}: {e}")
+            raise
 
     def get_diff_files(self) -> list[FilePatchInfo]:
         """
@@ -167,14 +204,9 @@ class GitLabProvider(GitProvider):
                 original_file_content_str = ''
                 new_file_content_str = ''
 
-            try:
+            # Ensure content is properly decoded
-                if isinstance(original_file_content_str, bytes):
+            original_file_content_str = decode_if_bytes(original_file_content_str)
-                    original_file_content_str = bytes.decode(original_file_content_str, 'utf-8')
+            new_file_content_str = decode_if_bytes(new_file_content_str)
-                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}")
 
             edit_type = EDIT_TYPE.MODIFIED
             if diff['new_file']:

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,12 +187,18 @@ 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 = ""
+            return
 
         if not self.changelog_file_str:
             self.changelog_file_str = self._get_default_changelog()

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 

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 == ""  # 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."""
+        # 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]}"