Widen scope of 'unmerged branches' to 'inactive branches'.

Rename the 'unmerged branches' metric to 'inactive branches' and enable it to also count branches that have been merged but not deleted.

Closes #1253.

components/api_server/src/initialization/migrations.py

@@ -22,6 +22,7 @@ def perform_migrations(database: Database) -> None:
                 remove_test_cases_manual_number(report),
                 change_ci_subject_types_to_development_environment(report),
                 change_sonarqube_parameters(report),
+                change_unmerged_branches_metrics_to_inactive_branches(report),
             ]
         ):
             change_description = " and to ".join([change for change in changes if change])
@@ -184,6 +185,18 @@ def change_sonarqube_parameter(
     return changed
 
 
+def change_unmerged_branches_metrics_to_inactive_branches(report) -> str:
+    """Change unmerged branches metrics to inactive branches metrics."""
+    # Added after Quality-time v5.19.0, see https://github.com/ICTU/quality-time/issues/1253
+    change = ""
+    for metric in metrics(report, ["unmerged_branches"]):
+        metric["type"] = "inactive_branches"
+        if not metric.get("name"):
+            metric["name"] = "Unmerged branches"
+        change = "metric type 'unmerged_branches' to 'inactive_branches'"
+    return change
+
+
 def log_unknown_parameter_values(value_mapping: dict[str, str], old_values: list[str], value_type: str, report) -> None:
     """Log old parameter values that do not exist in the mapping."""
     if unknown_values := [old_value for old_value in old_values if old_value not in value_mapping]:

components/api_server/tests/initialization/test_migrations.py

@@ -42,6 +42,10 @@ def inserted_report(self, **kwargs):
         del report["_id"]
         return report
 
+    def check_inserted_report(self, inserted_report):
+        """Check that the report was inserted."""
+        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+
 
 class NoOpMigrationTest(MigrationTestCase):
     """Unit tests for empty database and empty reports."""
@@ -80,18 +84,15 @@ def test_report_with_accessibility_metric(self):
         """Test that the migration succeeds with an accessibility metric."""
         self.database.reports.find.return_value = [self.existing_report(metric_type="accessibility")]
         perform_migrations(self.database)
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, self.inserted_report())
+        self.check_inserted_report(self.inserted_report())
 
     def test_accessibility_metric_with_name_and_unit(self):
         """Test that the migration succeeds with an accessibility metric, and existing name and unit are kept."""
         self.database.reports.find.return_value = [
             self.existing_report(metric_type="accessibility", metric_name="name", metric_unit="unit"),
         ]
         perform_migrations(self.database)
-        self.database.reports.replace_one.assert_called_once_with(
-            {"_id": "id"},
-            self.inserted_report(metric_name="name", metric_unit="unit"),
-        )
+        self.check_inserted_report(self.inserted_report(metric_name="name", metric_unit="unit"))
 
     def test_report_with_accessibility_metric_and_other_types(self):
         """Test that the migration succeeds with an accessibility metric and other metric types."""
@@ -103,7 +104,7 @@ def test_report_with_accessibility_metric_and_other_types(self):
         inserted_report = self.inserted_report()
         inserted_report["subjects"][SUBJECT_ID]["metrics"][METRIC_ID2] = {"type": "violations"}
         inserted_report["subjects"][SUBJECT_ID]["metrics"][METRIC_ID3] = {"type": "security_warnings"}
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(inserted_report)
 
 
 class BranchParameterTest(MigrationTestCase):
@@ -139,8 +140,7 @@ def test_report_with_branch_parameter_without_value(self):
             SOURCE_ID: {"type": "gitlab", "parameters": {"branch": "master"}},
             SOURCE_ID2: {"type": "cloc", "parameters": {"branch": ""}},
         }
-        inserted_report = self.inserted_report(metric_type="source_up_to_dateness", sources=inserted_sources)
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(self.inserted_report(metric_type="source_up_to_dateness", sources=inserted_sources))
 
 
 class SourceParameterHashMigrationTest(MigrationTestCase):
@@ -196,7 +196,7 @@ def test_report_with_ci_environment(self):
         inserted_report = self.inserted_report(metric_type="failed_jobs")
         inserted_report["subjects"][SUBJECT_ID]["name"] = "CI-environment"
         inserted_report["subjects"][SUBJECT_ID]["description"] = "A continuous integration environment."
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(inserted_report)
 
     def test_ci_environment_with_title_and_subtitle(self):
         """Test that the migration succeeds with an CI-environment subject, and existing title and subtitle are kept."""
@@ -209,7 +209,7 @@ def test_ci_environment_with_title_and_subtitle(self):
         inserted_report = self.inserted_report(metric_type="failed_jobs")
         inserted_report["subjects"][SUBJECT_ID]["name"] = "CI"
         inserted_report["subjects"][SUBJECT_ID]["description"] = "My CI"
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(inserted_report)
 
 
 class SonarQubeParameterTest(MigrationTestCase):
@@ -245,7 +245,7 @@ def test_report_with_severity_parameter(self):
             metric_type="violations",
             sources=self.sources(impact_severities=["low"]),
         )
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(inserted_report)
 
     def test_report_with_multiple_old_severity_values_that_map_to_the_same_new_value(self):
         """Test a severity parameter with multiple old values that map to the same new value."""
@@ -256,7 +256,7 @@ def test_report_with_multiple_old_severity_values_that_map_to_the_same_new_value
             metric_type="violations",
             sources=self.sources(impact_severities=["low"]),
         )
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(inserted_report)
 
     @disable_logging
     def test_report_with_unknown_old_severity_values(self):
@@ -268,7 +268,7 @@ def test_report_with_unknown_old_severity_values(self):
         inserted_sources = self.sources(impact_severities=["low"])
         inserted_sources[SOURCE_ID2] = {"type": "sonarqube", "parameters": {"branch": "main"}}
         inserted_report = self.inserted_report(metric_type="violations", sources=inserted_sources)
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(inserted_report)
 
     def test_report_with_types_parameter(self):
         """Test that the migration succeeds when the SonarQube source has a types parameter."""
@@ -280,16 +280,15 @@ def test_report_with_types_parameter(self):
             metric_type="violations",
             sources=self.sources(impacted_software_qualities=["reliability"]),
         )
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(inserted_report)
 
     def test_report_with_types_parameter_without_values(self):
         """Test that the migration succeeds when the SonarQube source has a types parameter without values."""
         self.database.reports.find.return_value = [
             self.existing_report(metric_type="violations", sources=self.sources(types=[])),
         ]
         perform_migrations(self.database)
-        inserted_report = self.inserted_report(metric_type="violations", sources=self.sources())
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(self.inserted_report(metric_type="violations", sources=self.sources()))
 
     def test_report_with_security_types_parameter(self):
         """Test that the migration succeeds when the SonarQube source has a security types parameter."""
@@ -302,7 +301,7 @@ def test_report_with_security_types_parameter(self):
         perform_migrations(self.database)
         inserted_sources = self.sources(security_types=["issue with security impact", "security hotspot"])
         inserted_report = self.inserted_report(metric_type="security_warnings", sources=inserted_sources)
-        self.database.reports.replace_one.assert_called_once_with({"_id": "id"}, inserted_report)
+        self.check_inserted_report(inserted_report)
 
     def test_report_with_security_types_parameter_without_values(self):
         """Test that the migration succeeds when the SonarQube source has a security types parameter without values."""
@@ -336,18 +335,48 @@ def test_report_with_test_cases_and_manual_number_source(self):
             self.existing_report(metric_type="test_cases", sources=self.sources()),
         ]
         perform_migrations(self.database)
-        self.database.reports.replace_one.assert_called_once_with(
-            {"_id": "id"},
-            self.inserted_report(metric_type="test_cases", sources={}),
-        )
+        self.check_inserted_report(self.inserted_report(metric_type="test_cases", sources={}))
 
     def test_report_with_test_cases_and_jira_and_manual_number_source(self):
         """Test that the manual number source is removed."""
         self.database.reports.find.return_value = [
             self.existing_report(metric_type="test_cases", sources=self.sources("jira", "manual_number")),
         ]
         perform_migrations(self.database)
-        self.database.reports.replace_one.assert_called_once_with(
-            {"_id": "id"},
-            self.inserted_report(metric_type="test_cases", sources=self.sources("jira")),
+        self.check_inserted_report(self.inserted_report(metric_type="test_cases", sources=self.sources("jira")))
+
+
+class UnmergedToInactiveBranchesTest(MigrationTestCase):
+    """Unit tests for the unmerged to inactive branches migration."""
+
+    def setUp(self):
+        """Create test fixture."""
+        super().setUp()
+        self.sources = {SOURCE_ID: {"type": "gitlab"}}
+
+    def test_report_with_unmerged_branches_metric(self):
+        """Test that an unmerged branches metric is migrated."""
+        self.database.reports.find.return_value = [
+            self.existing_report(metric_type="unmerged_branches", sources=self.sources),
+        ]
+        perform_migrations(self.database)
+        inserted_report = self.inserted_report(
+            metric_type="inactive_branches",
+            metric_name="Unmerged branches",
+            sources=self.sources,
+        )
+        self.check_inserted_report(inserted_report)
+
+    def test_report_with_unmerged_branches_metric_with_name(self):
+        """Test that the metric name is not changed when migrating the type."""
+        metric_name = "Old branches"
+        self.database.reports.find.return_value = [
+            self.existing_report(metric_type="unmerged_branches", metric_name=metric_name, sources=self.sources),
+        ]
+        perform_migrations(self.database)
+        inserted_report = self.inserted_report(
+            metric_type="inactive_branches",
+            metric_name=metric_name,
+            sources=self.sources,
         )
+        self.check_inserted_report(inserted_report)

components/collector/.vulture_ignore_list.py

@@ -14,7 +14,7 @@
 AzureDevopsJobRunsWithinTimePeriod  # unused class (src/source_collectors/azure_devops/job_runs_within_time_period.py:11)
 AzureDevopsSourceUpToDateness  # unused class (src/source_collectors/azure_devops/source_up_to_dateness.py:53)
 __new__  # unused function (src/source_collectors/azure_devops/source_up_to_dateness.py:56)
-AzureDevopsUnmergedBranches  # unused class (src/source_collectors/azure_devops/unmerged_branches.py:15)
+AzureDevopsInactiveBranches  # unused class (src/source_collectors/azure_devops/unmerged_branches.py:15)
 AzureDevopsUnusedJobs  # unused class (src/source_collectors/azure_devops/unused_jobs.py:11)
 AzureDevopsUserStoryPoints  # unused class (src/source_collectors/azure_devops/user_story_points.py:12)
 BanditSecurityWarnings  # unused class (src/source_collectors/bandit/security_warnings.py:10)
@@ -61,7 +61,7 @@
 GitLabSourceUpToDateness  # unused class (src/source_collectors/gitlab/source_up_to_dateness.py:93)
 __new__  # unused function (src/source_collectors/gitlab/source_up_to_dateness.py:96)
 GitLabSourceVersion  # unused class (src/source_collectors/gitlab/source_version.py:11)
-GitLabUnmergedBranches  # unused class (src/source_collectors/gitlab/unmerged_branches.py:15)
+GitLabInactiveBranches  # unused class (src/source_collectors/gitlab/unmerged_branches.py:15)
 GitLabUnusedJobs  # unused class (src/source_collectors/gitlab/unused_jobs.py:11)
 scan_status  # unused variable (src/source_collectors/harbor/security_warnings.py:58)
 HarborSecurityWarnings  # unused class (src/source_collectors/harbor/security_warnings.py:62)

components/collector/src/base_collectors/__init__.py

@@ -11,13 +11,13 @@
 )
 from .metric_collector import MetricCollector
 from .source_collector import (
+    InactiveBranchesSourceCollector,
     SecurityWarningsSourceCollector,
     SlowTransactionsCollector,
     SourceCollector,
     SourceMeasurement,
     TimePassedCollector,
     TimeRemainingCollector,
     TransactionEntity,
-    UnmergedBranchesSourceCollector,
     VersionCollector,
 )

components/collector/src/base_collectors/source_collector.py

@@ -219,24 +219,24 @@ async def _landing_url(self, responses: SourceResponses) -> URL:
         return URL(url.removesuffix("xml") + "html" if url.endswith(".xml") else url)
 
 
-class UnmergedBranchesSourceCollector(SourceCollector, ABC):
-    """Base class for unmerged branches source collectors."""
+class InactiveBranchesSourceCollector(SourceCollector, ABC):
+    """Base class for inactive branches source collectors."""
 
     async def _parse_entities(self, responses: SourceResponses) -> Entities:
-        """Override to get the unmerged branches from the unmerged branches method that subclasses should implement."""
+        """Override to get the inactive branches from the inactive branches method that subclasses should implement."""
         return Entities(
             Entity(
                 key=branch["name"],
                 name=branch["name"],
                 commit_date=str(self._commit_datetime(branch).date()),
                 url=str(self._branch_landing_url(branch)),
             )
-            for branch in await self._unmerged_branches(responses)
+            for branch in await self._inactive_branches(responses)
         )
 
     @abstractmethod
-    async def _unmerged_branches(self, responses: SourceResponses) -> list[dict[str, Any]]:
-        """Return the list of unmerged branches."""
+    async def _inactive_branches(self, responses: SourceResponses) -> list[dict[str, Any]]:
+        """Return the list of inactive branches."""
 
     @abstractmethod
     def _commit_datetime(self, branch) -> datetime:

components/collector/src/source_collectors/__init__.py

@@ -12,12 +12,12 @@
 from .azure_devops.average_issue_lead_time import AzureDevopsAverageIssueLeadTime
 from .azure_devops.change_failure_rate import AzureDevopsChangeFailureRate
 from .azure_devops.failed_jobs import AzureDevopsFailedJobs
+from .azure_devops.inactive_branches import AzureDevopsInactiveBranches
 from .azure_devops.issues import AzureDevopsIssues
 from .azure_devops.job_runs_within_time_period import AzureDevopsJobRunsWithinTimePeriod
 from .azure_devops.merge_requests import AzureDevopsMergeRequests
 from .azure_devops.source_up_to_dateness import AzureDevopsSourceUpToDateness
 from .azure_devops.tests import AzureDevopsTests
-from .azure_devops.unmerged_branches import AzureDevopsUnmergedBranches
 from .azure_devops.unused_jobs import AzureDevopsUnusedJobs
 from .azure_devops.user_story_points import AzureDevopsUserStoryPoints
 from .bandit.security_warnings import BanditSecurityWarnings
@@ -51,12 +51,12 @@
 from .github.merge_requests import GitHubMergeRequests
 from .gitlab.change_failure_rate import GitLabChangeFailureRate
 from .gitlab.failed_jobs import GitLabFailedJobs
+from .gitlab.inactive_branches import GitLabInactiveBranches
 from .gitlab.job_runs_within_time_period import GitLabJobRunsWithinTimePeriod
 from .gitlab.merge_requests import GitLabMergeRequests
 from .gitlab.pipeline_duration import GitLabPipelineDuration
 from .gitlab.source_up_to_dateness import GitLabSourceUpToDateness
 from .gitlab.source_version import GitLabSourceVersion
-from .gitlab.unmerged_branches import GitLabUnmergedBranches
 from .gitlab.unused_jobs import GitLabUnusedJobs
 from .harbor.security_warnings import HarborSecurityWarnings
 from .harbor_json.security_warnings import HarborJSONSecurityWarnings

components/collector/src/source_collectors/azure_devops/unmerged_branches.py → components/collector/src/source_collectors/azure_devops/inactive_branches.py

@@ -1,9 +1,9 @@
-"""Azure DevOps Server unmerged branches collector."""
+"""Azure DevOps Server inactive branches collector."""
 
 from datetime import datetime
 from typing import Any, cast
 
-from base_collectors import UnmergedBranchesSourceCollector
+from base_collectors import InactiveBranchesSourceCollector
 from collector_utilities.date_time import days_ago, parse_datetime
 from collector_utilities.functions import match_string_or_regular_expression
 from collector_utilities.type import URL
@@ -12,8 +12,8 @@
 from .base import AzureDevopsRepositoryBase
 
 
-class AzureDevopsUnmergedBranches(UnmergedBranchesSourceCollector, AzureDevopsRepositoryBase):
-    """Collector for unmerged branches."""
+class AzureDevopsInactiveBranches(InactiveBranchesSourceCollector, AzureDevopsRepositoryBase):
+    """Collector for inactive branches."""
 
     async def _api_url(self) -> URL:
         """Extend to add the branches API path."""
@@ -25,11 +25,12 @@ async def _landing_url(self, responses: SourceResponses) -> URL:
         landing_url = str(await super()._landing_url(responses))
         return URL(f"{landing_url}/branches")
 
-    async def _unmerged_branches(self, responses: SourceResponses) -> list[dict[str, Any]]:
-        """Override to get the unmerged branches response.
+    async def _inactive_branches(self, responses: SourceResponses) -> list[dict[str, Any]]:
+        """Override to get the inactive branches response.
 
-        Branches are considered unmerged if they have a base branch, have commits that are not on the base branch,
-        have not been committed to for a minimum number of days, and are not to be ignored.
+        Branches are considered inactive if are unmerged (meaning they have a base branch, have commits that are not
+        on the base branch, have not been committed to for a minimum number of days) or  merged (all commits are on
+        the base branch, have not been committed to for a minimum number of days).
         """
         return [
             branch

components/collector/src/source_collectors/gitlab/unmerged_branches.py → components/collector/src/source_collectors/gitlab/inactive_branches.py

@@ -1,9 +1,9 @@
-"""GitLab unmerged branches collector."""
+"""GitLab inactive branches collector."""
 
 from datetime import datetime
 from typing import Any, cast
 
-from base_collectors import UnmergedBranchesSourceCollector
+from base_collectors import InactiveBranchesSourceCollector
 from collector_utilities.date_time import days_ago, parse_datetime
 from collector_utilities.functions import match_string_or_regular_expression
 from collector_utilities.type import URL
@@ -12,7 +12,7 @@
 from .base import GitLabProjectBase
 
 
-class GitLabUnmergedBranches(GitLabProjectBase, UnmergedBranchesSourceCollector):
+class GitLabInactiveBranches(GitLabProjectBase, InactiveBranchesSourceCollector):
     """Collector class to measure the number of unmerged branches."""
 
     async def _api_url(self) -> URL:
@@ -23,8 +23,8 @@ async def _landing_url(self, responses: SourceResponses) -> URL:
         """Extend to add the project branches."""
         return URL(f"{await super()._landing_url(responses)}/{self._parameter('project')}/-/branches")
 
-    async def _unmerged_branches(self, responses: SourceResponses) -> list[dict[str, Any]]:
-        """Override to return a list of unmerged and inactive branches."""
+    async def _inactive_branches(self, responses: SourceResponses) -> list[dict[str, Any]]:
+        """Override to return a list of inactive branches."""
         branches = []
         for response in responses:
             branches.extend(await response.json())