From 9ad11d523576b2e43312c9454bab0f82f7b90519 Mon Sep 17 00:00:00 2001 From: Frank Niessink Date: Thu, 21 Nov 2024 15:25:04 +0100 Subject: [PATCH] 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. --- .../src/initialization/migrations.py | 13 ++++ .../tests/initialization/test_migrations.py | 77 +++++++++++++------ components/collector/.vulture_ignore_list.py | 4 +- .../collector/src/base_collectors/__init__.py | 2 +- .../src/base_collectors/source_collector.py | 12 +-- .../src/source_collectors/__init__.py | 4 +- ...erged_branches.py => inactive_branches.py} | 17 ++-- ...erged_branches.py => inactive_branches.py} | 10 +-- ..._branches.py => test_inactive_branches.py} | 10 +-- ..._branches.py => test_inactive_branches.py} | 8 +- .../src/shared_data_model/metrics.py | 39 +++++----- .../src/shared_data_model/parameters.py | 13 +++- .../shared_data_model/sources/azure_devops.py | 10 ++- .../src/shared_data_model/sources/gitlab.py | 10 ++- .../sources/manual_number.py | 2 +- .../shared_data_model/sources/quality_time.py | 4 +- .../src/shared_data_model/subjects.py | 2 +- docs/src/changelog.md | 6 ++ docs/src/versioning.md | 19 ++--- 19 files changed, 164 insertions(+), 98 deletions(-) rename components/collector/src/source_collectors/azure_devops/{unmerged_branches.py => inactive_branches.py} (70%) rename components/collector/src/source_collectors/gitlab/{unmerged_branches.py => inactive_branches.py} (84%) rename components/collector/tests/source_collectors/azure_devops/{test_unmerged_branches.py => test_inactive_branches.py} (88%) rename components/collector/tests/source_collectors/gitlab/{test_unmerged_branches.py => test_inactive_branches.py} (92%) diff --git a/components/api_server/src/initialization/migrations.py b/components/api_server/src/initialization/migrations.py index 0f3274df28..558e29eb8d 100644 --- a/components/api_server/src/initialization/migrations.py +++ b/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]: diff --git a/components/api_server/tests/initialization/test_migrations.py b/components/api_server/tests/initialization/test_migrations.py index c4fa36008e..b1697c5a3c 100644 --- a/components/api_server/tests/initialization/test_migrations.py +++ b/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,7 +84,7 @@ 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.""" @@ -88,10 +92,7 @@ def test_accessibility_metric_with_name_and_unit(self): 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,7 +280,7 @@ 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.""" @@ -288,8 +288,7 @@ def test_report_with_types_parameter_without_values(self): 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,10 +335,7 @@ 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.""" @@ -347,7 +343,40 @@ def test_report_with_test_cases_and_jira_and_manual_number_source(self): 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) diff --git a/components/collector/.vulture_ignore_list.py b/components/collector/.vulture_ignore_list.py index 8e2c459e4e..53fefcf4d1 100644 --- a/components/collector/.vulture_ignore_list.py +++ b/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) diff --git a/components/collector/src/base_collectors/__init__.py b/components/collector/src/base_collectors/__init__.py index f70bee4981..fb9b594e2a 100644 --- a/components/collector/src/base_collectors/__init__.py +++ b/components/collector/src/base_collectors/__init__.py @@ -11,6 +11,7 @@ ) from .metric_collector import MetricCollector from .source_collector import ( + InactiveBranchesSourceCollector, SecurityWarningsSourceCollector, SlowTransactionsCollector, SourceCollector, @@ -18,6 +19,5 @@ TimePassedCollector, TimeRemainingCollector, TransactionEntity, - UnmergedBranchesSourceCollector, VersionCollector, ) diff --git a/components/collector/src/base_collectors/source_collector.py b/components/collector/src/base_collectors/source_collector.py index 59aad99fcd..912e154eb5 100644 --- a/components/collector/src/base_collectors/source_collector.py +++ b/components/collector/src/base_collectors/source_collector.py @@ -219,11 +219,11 @@ 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"], @@ -231,12 +231,12 @@ async def _parse_entities(self, responses: SourceResponses) -> Entities: 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: diff --git a/components/collector/src/source_collectors/__init__.py b/components/collector/src/source_collectors/__init__.py index 0efd217470..ac16a1af49 100644 --- a/components/collector/src/source_collectors/__init__.py +++ b/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 diff --git a/components/collector/src/source_collectors/azure_devops/unmerged_branches.py b/components/collector/src/source_collectors/azure_devops/inactive_branches.py similarity index 70% rename from components/collector/src/source_collectors/azure_devops/unmerged_branches.py rename to components/collector/src/source_collectors/azure_devops/inactive_branches.py index bf9a21407b..9aa3089dcc 100644 --- a/components/collector/src/source_collectors/azure_devops/unmerged_branches.py +++ b/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 diff --git a/components/collector/src/source_collectors/gitlab/unmerged_branches.py b/components/collector/src/source_collectors/gitlab/inactive_branches.py similarity index 84% rename from components/collector/src/source_collectors/gitlab/unmerged_branches.py rename to components/collector/src/source_collectors/gitlab/inactive_branches.py index 35d26a629e..c8632166ba 100644 --- a/components/collector/src/source_collectors/gitlab/unmerged_branches.py +++ b/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()) diff --git a/components/collector/tests/source_collectors/azure_devops/test_unmerged_branches.py b/components/collector/tests/source_collectors/azure_devops/test_inactive_branches.py similarity index 88% rename from components/collector/tests/source_collectors/azure_devops/test_unmerged_branches.py rename to components/collector/tests/source_collectors/azure_devops/test_inactive_branches.py index 48eb3f4970..ad348c99ab 100644 --- a/components/collector/tests/source_collectors/azure_devops/test_unmerged_branches.py +++ b/components/collector/tests/source_collectors/azure_devops/test_inactive_branches.py @@ -1,12 +1,12 @@ -"""Unit tests for the Azure DevOps Server unmerged branches collector.""" +"""Unit tests for the Azure DevOps Server inactive branches collector.""" from .base import AzureDevopsTestCase -class AzureDevopsUnmergedBranchesTest(AzureDevopsTestCase): - """Unit tests for the Azure DevOps Server unmerged branches.""" +class AzureDevopsInactiveBranchesTest(AzureDevopsTestCase): + """Unit tests for the Azure DevOps Server inactive branches.""" - METRIC_TYPE = "unmerged_branches" + METRIC_TYPE = "inactive_branches" def setUp(self): """Extend to set up the metric under test.""" @@ -16,7 +16,7 @@ def setUp(self): self.landing_url = f"{self.url}/_git/project/branches" async def test_no_branches_except_master(self): - """Test that the number of unmerged branches is returned.""" + """Test that the number of inactive branches is returned.""" branches = {"value": [{"name": "main", "isBaseVersion": True}]} response = await self.collect(get_request_json_side_effect=[self.repositories, branches]) self.assert_measurement(response, value="0", entities=[], landing_url=self.landing_url) diff --git a/components/collector/tests/source_collectors/gitlab/test_unmerged_branches.py b/components/collector/tests/source_collectors/gitlab/test_inactive_branches.py similarity index 92% rename from components/collector/tests/source_collectors/gitlab/test_unmerged_branches.py rename to components/collector/tests/source_collectors/gitlab/test_inactive_branches.py index cc1c049ce8..1fb8537579 100644 --- a/components/collector/tests/source_collectors/gitlab/test_unmerged_branches.py +++ b/components/collector/tests/source_collectors/gitlab/test_inactive_branches.py @@ -1,4 +1,4 @@ -"""Unit tests for the GitLab unmerged branches collector.""" +"""Unit tests for the GitLab inactive branches collector.""" from datetime import datetime @@ -7,10 +7,10 @@ from .base import GitLabTestCase -class GitLabUnmergedBranchesTest(GitLabTestCase): - """Unit tests for the unmerged branches metric.""" +class GitLabInactiveBranchesTest(GitLabTestCase): + """Unit tests for the inactive branches metric.""" - METRIC_TYPE = "unmerged_branches" + METRIC_TYPE = "inactive_branches" def setUp(self): """Extend to setup fixtures.""" diff --git a/components/shared_code/src/shared_data_model/metrics.py b/components/shared_code/src/shared_data_model/metrics.py index 18ea601a6b..2cbc5afb14 100644 --- a/components/shared_code/src/shared_data_model/metrics.py +++ b/components/shared_code/src/shared_data_model/metrics.py @@ -118,6 +118,26 @@ sources=["azure_devops", "jenkins", "gitlab", "manual_number"], tags=[Tag.CI], ), + "inactive_branches": Metric( + name="Inactive branches", + description="The number of branches that have been inactive for a while.", + rationale="It is strange if branches have had no activity for a while and have not been merged to the " + "default branch. Maybe commits have been cherry picked, or maybe the work has been postponed, but it also " + "sometimes happen that someone simply forgets to merge the branch. Likewise, it is strange if branches " + "are not deleted after having been merged to the default branch.", + documentation="""To change how soon *Quality-time* should consider branches to be inactive, use the + parameter "Number of days since last commit after which to consider branches inactive". + + What exactly is the default branch is configured in GitLab or Azure DevOps. If you want to use a different branch + as default branch, you need to configure this in the source, see the documentation for + [GitLab](https://docs.gitlab.com/ee/user/project/repository/branches/default.html) or + [Azure DevOps](https://docs.microsoft.com/en-us/azure/devops/repos/git/manage-your-branches?view=azure-devops#\ + change-your-default-branch).""", + unit=Unit.BRANCHES, + near_target="5", + sources=["azure_devops", "gitlab", "manual_number"], + tags=[Tag.CI], + ), "issues": Metric( name="Issues", description="The number of issues.", @@ -670,25 +690,6 @@ ], tags=[Tag.TEST_QUALITY], ), - "unmerged_branches": Metric( - name="Unmerged branches", - description="The number of branches that have not been merged to the default branch.", - rationale="It is strange if branches have had no activity for a while and have not been merged to the " - "default branch. Maybe commits have been cherry picked, or maybe the work has been postponed, but it " - "also sometimes happen that someone simply forgets to merge the branch.", - documentation="""To change how soon *Quality-time* should consider branches to be inactive, use the -parameter "Number of days since last commit after which to consider branches inactive". - -What exactly is the default branch is configured in GitLab or Azure DevOps. If you want to use a different branch -as default branch, you need to configure this in the source, see the documentation for -[GitLab](https://docs.gitlab.com/ee/user/project/repository/branches/default.html) or -[Azure DevOps](https://docs.microsoft.com/en-us/azure/devops/repos/git/manage-your-branches?view=azure-devops#\ -change-your-default-branch).""", - unit=Unit.BRANCHES, - near_target="5", - sources=["azure_devops", "gitlab", "manual_number"], - tags=[Tag.CI], - ), "unused_jobs": Metric( name="Unused CI-jobs", description="The number of continuous integration jobs that are unused.", diff --git a/components/shared_code/src/shared_data_model/parameters.py b/components/shared_code/src/shared_data_model/parameters.py index 2c70a4832d..5dde3bc3d2 100644 --- a/components/shared_code/src/shared_data_model/parameters.py +++ b/components/shared_code/src/shared_data_model/parameters.py @@ -165,7 +165,7 @@ class BranchesToIgnore(MultipleChoiceWithAdditionParameter): name: str = "Branches to ignore (regular expressions or branch names)" short_name: str = "branches to ignore" - metrics: list[str] = ["unmerged_branches"] + metrics: list[str] = ["inactive_branches"] class TargetBranchesToInclude(MultipleChoiceWithAdditionParameter): @@ -177,6 +177,17 @@ class TargetBranchesToInclude(MultipleChoiceWithAdditionParameter): metrics: list[str] = ["merge_requests"] +class BranchMergeStatus(MultipleChoiceParameter): + """Branch merge status.""" + + name: str = "Branch merge status" + short_name: str = "merge status" + help: str = "Limit which merge states to count." + placeholder: str = "all merge states" + metrics: list[str] = ["inactive_branches"] + values: list[str] = ["merged", "unmerged"] + + class MergeRequestState(MultipleChoiceParameter): """Merge request states parameter.""" diff --git a/components/shared_code/src/shared_data_model/sources/azure_devops.py b/components/shared_code/src/shared_data_model/sources/azure_devops.py index 70d9539818..37a0ffee64 100644 --- a/components/shared_code/src/shared_data_model/sources/azure_devops.py +++ b/components/shared_code/src/shared_data_model/sources/azure_devops.py @@ -8,6 +8,7 @@ URL, Branch, BranchesToIgnore, + BranchMergeStatus, Days, FailureType, MergeRequestState, @@ -25,12 +26,12 @@ "average_issue_lead_time", "change_failure_rate", "failed_jobs", + "inactive_branches", "issues", "job_runs_within_time_period", "merge_requests", "source_up_to_dateness", "tests", - "unmerged_branches", "unused_jobs", "user_story_points", ] @@ -103,7 +104,7 @@ name="Repository (name or id)", short_name="repository", placeholder="default repository", - metrics=["merge_requests", "source_up_to_dateness", "unmerged_branches"], + metrics=["inactive_branches", "merge_requests", "source_up_to_dateness"], ), "branch": Branch(), "branches_to_ignore": BranchesToIgnore( @@ -111,11 +112,12 @@ "https://docs.microsoft.com/en-us/azure/devops/repos/git/manage-your-branches?view=azure-devops", ), ), + "branch_merge_status": BranchMergeStatus(), "inactive_days": Days( name="Number of days since last commit after which to consider branches inactive", short_name="number of days since last commit", default_value="7", - metrics=["unmerged_branches"], + metrics=["inactive_branches"], ), "inactive_job_days": Days( name="Number of days since last build after which to consider pipelines inactive", @@ -262,7 +264,7 @@ ], ), "unused_jobs": Entity(name="unused pipeline", attributes=PIPELINE_ATTRIBUTES), - "unmerged_branches": Entity( + "inactive_branches": Entity( name="branch", name_plural="branches", attributes=[ diff --git a/components/shared_code/src/shared_data_model/sources/gitlab.py b/components/shared_code/src/shared_data_model/sources/gitlab.py index c04c8ff571..7a7813de5f 100644 --- a/components/shared_code/src/shared_data_model/sources/gitlab.py +++ b/components/shared_code/src/shared_data_model/sources/gitlab.py @@ -9,6 +9,7 @@ Branch, Branches, BranchesToIgnore, + BranchMergeStatus, Days, FailureType, MergeRequestState, @@ -24,12 +25,12 @@ ALL_GITLAB_METRICS = [ "change_failure_rate", "failed_jobs", + "inactive_branches", "job_runs_within_time_period", "merge_requests", "pipeline_duration", "source_up_to_dateness", "source_version", - "unmerged_branches", "unused_jobs", ] @@ -110,11 +111,11 @@ metrics=[ "change_failure_rate", "failed_jobs", + "inactive_branches", "job_runs_within_time_period", "pipeline_duration", "merge_requests", "source_up_to_dateness", - "unmerged_branches", "unused_jobs", ], ), @@ -133,6 +134,7 @@ "branch": Branch(help_url=GITLAB_BRANCH_HELP_URL), "branches": Branches(help_url=GITLAB_BRANCH_HELP_URL), "branches_to_ignore": BranchesToIgnore(help_url=GITLAB_BRANCH_HELP_URL), + "branch_merge_status": BranchMergeStatus(), "refs_to_ignore": MultipleChoiceWithAdditionParameter( name="Branches and tags to ignore (regular expressions, branch names or tag names)", short_name="branches and tags to ignore", @@ -143,7 +145,7 @@ name="Number of days since last commit after which to consider branches inactive", short_name="number of days since last commit", default_value="7", - metrics=["unmerged_branches"], + metrics=["inactive_branches"], ), "inactive_job_days": Days( name="Number of days without builds after which to consider CI-jobs unused", @@ -267,7 +269,7 @@ EntityAttribute(name="Merged", type=EntityAttributeType.DATETIME), ], ), - "unmerged_branches": Entity( + "inactive_branches": Entity( name="branch", name_plural="branches", attributes=[ diff --git a/components/shared_code/src/shared_data_model/sources/manual_number.py b/components/shared_code/src/shared_data_model/sources/manual_number.py index 173d787dee..d72f7f5759 100644 --- a/components/shared_code/src/shared_data_model/sources/manual_number.py +++ b/components/shared_code/src/shared_data_model/sources/manual_number.py @@ -26,6 +26,7 @@ "dependencies", "duplicated_lines", "failed_jobs", + "inactive_branches", "issues", "job_runs_within_time_period", "loc", @@ -49,7 +50,6 @@ "todo_and_fixme_comments", "uncovered_branches", "uncovered_lines", - "unmerged_branches", "unused_jobs", "user_story_points", "velocity", diff --git a/components/shared_code/src/shared_data_model/sources/quality_time.py b/components/shared_code/src/shared_data_model/sources/quality_time.py index d822725fc9..ebfa1d3d6e 100644 --- a/components/shared_code/src/shared_data_model/sources/quality_time.py +++ b/components/shared_code/src/shared_data_model/sources/quality_time.py @@ -81,6 +81,7 @@ "Dependencies", "Duplicated lines", "Failed CI-jobs", + "Inactive branches", "Issues", "Job runs within time period", "Violation remediation effort", @@ -107,7 +108,6 @@ "Tests", "Time remaining", "Todo and fixme comments", - "Unmerged branches", "Unused CI-jobs", "User story points", "Velocity", @@ -123,6 +123,7 @@ "Dependencies": "dependencies", "Duplicated lines": "duplicated_lines", "Failed CI-jobs": "failed_jobs", + "Inactive branches": "inactive_branches", "Issues": "issues", "Job runs within time period": "job_runs_within_time_period", "Long units": "long_units", @@ -148,7 +149,6 @@ "Tests": "tests", "Time remaining": "time_remaining", "Todo and fixme comments": "todo_and_fixme_comments", - "Unmerged branches": "unmerged_branches", "Unused CI-jobs": "unused_jobs", "User story points": "user_story_points", "Velocity": "velocity", diff --git a/components/shared_code/src/shared_data_model/subjects.py b/components/shared_code/src/shared_data_model/subjects.py index a1eb8e33d6..c46d9e9586 100644 --- a/components/shared_code/src/shared_data_model/subjects.py +++ b/components/shared_code/src/shared_data_model/subjects.py @@ -46,9 +46,9 @@ name="Development process", description="A software development and/or maintenance process.", metrics=[ + "inactive_branches", "merge_requests", "pipeline_duration", - "unmerged_branches", ], ), "process_operations": Subject( diff --git a/docs/src/changelog.md b/docs/src/changelog.md index 2333111e84..e607c94718 100644 --- a/docs/src/changelog.md +++ b/docs/src/changelog.md @@ -12,6 +12,12 @@ If your currently installed *Quality-time* version is not the latest version, pl +## [Unreleased] + +### Changed + +- Rename the 'unmerged branches' metric to 'inactive branches' and enable it to also count branches that have been merged but not deleted. Closes [#1253](https://github.com/ICTU/quality-time/issues/1253). + ## v5.19.0 - 2024-11-22 ### Added diff --git a/docs/src/versioning.md b/docs/src/versioning.md index 74bb05d9ff..5326dedf72 100644 --- a/docs/src/versioning.md +++ b/docs/src/versioning.md @@ -21,15 +21,16 @@ The table below contains the *Quality-time* releases since the last minor of the | Version | Date | Mongo | FC | Migrations | Downgrade | Upgrade | Manual changes | |------------|--------------|--------|--------|------------|-----------------|-----------------|----------------| -| v5.19.0 | 2024-11-22 | v8 | v7 | | v5.14.0-v5.18.0 | n/a | no | -| v5.18.0 | 2024-11-06 | v8 | v7 | | v5.14.0-v5.17.1 | v5.19.0 | no | -| v5.17.1 | 2024-10-25 | v8 | v7 | | v5.14.0-v5.17.0 | v5.18.0-v5.19.0 | no | -| v5.17.0 | 2024-10-17 | **v8** | v7 | | v5.14.0-v5.16.2 | v5.17.1-v5.19.0 | no | -| v5.16.2 | 2024-10-03 | v7 | v7 | | v5.14.0-v5.16.1 | v5.17.0-v5.19.0 | no | -| v5.16.1 | 2024-09-26 | v7 | v7 | | v5.14.0-v5.16.0 | v5.16.2-v5.19.0 | no | -| v5.16.0 | 2024-09-19 | v7 | v7 | added | v5.14.0-v5.15.0 | v5.16.1-v5.19.0 | no | -| v5.15.0 | 2024-07-30 | v7 | v7 | | v5.14.0 | v5.16.0-v5.19.0 | no | -| v5.14.0 | 2024-07-05 | v7 | **v7** | added | not possible | v5.15.0-v5.19.0 | no | +| v5.20.0 | [unreleased] | v8 | v7 | added | not possible | n/a | no | +| v5.19.0 | 2024-11-22 | v8 | v7 | | v5.14.0-v5.18.0 | v5.20.0 | no | +| v5.18.0 | 2024-11-06 | v8 | v7 | | v5.14.0-v5.17.1 | v5.19.0-v5.20.0 | no | +| v5.17.1 | 2024-10-25 | v8 | v7 | | v5.14.0-v5.17.0 | v5.18.0-v5.20.0 | no | +| v5.17.0 | 2024-10-17 | **v8** | v7 | | v5.14.0-v5.16.2 | v5.17.1-v5.20.0 | no | +| v5.16.2 | 2024-10-03 | v7 | v7 | | v5.14.0-v5.16.1 | v5.17.0-v5.20.0 | no | +| v5.16.1 | 2024-09-26 | v7 | v7 | | v5.14.0-v5.16.0 | v5.16.2-v5.20.0 | no | +| v5.16.0 | 2024-09-19 | v7 | v7 | added | v5.14.0-v5.15.0 | v5.16.1-v5.20.0 | no | +| v5.15.0 | 2024-07-30 | v7 | v7 | | v5.14.0 | v5.16.0-v5.20.0 | no | +| v5.14.0 | 2024-07-05 | v7 | **v7** | added | not possible | v5.15.0-v5.20.0 | no | | v5.13.0 | 2024-05-23 | v7 | v6 | added | not possible | v5.14.0-v5.16.2 | no | | v5.12.0 | 2024-05-17 | v7 | v6 | added | not possible | v5.13.0-v5.16.2 | no | | v5.11.0 | 2024-04-22 | v7 | v6 | | v5.6.0-v5.10.0 | v5.12.0-v5.16.2 | no |