From 4de0b7beeb19f954921df1647a517b95f3142b47 Mon Sep 17 00:00:00 2001 From: Shihao Date: Thu, 1 Oct 2026 21:50:18 -0600 Subject: [PATCH] Always auto-move patches tagged Bugfix when a commitfest closes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug fixes that had a quiet thread or a red CI stayed behind in the closed commitfest, where nobody looks for them. Move every open patch tagged Bugfix to the next commitfest, whatever its activity or CI state. Requested by Álvaro Herrera: https://www.postgresql.org/message-id/ar4QFqNuUX-e0v2B%40alvherre.pgsql --- pgcommitfest/commitfest/models.py | 25 +++-- pgcommitfest/commitfest/templates/help.html | 2 +- .../tests/test_closure_notifications.py | 95 +++++++++++++++++++ 3 files changed, 115 insertions(+), 7 deletions(-) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 2d21a1ec..4f0a57b5 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -11,6 +11,10 @@ from .util import DiffableModel +# Patches with one of these tags are always moved to the next commitfest when +# their commitfest closes. +AUTO_MOVE_ALWAYS_TAG_NAMES = ["Bugfix"] + # We have few enough of these, and it's really the only thing we # need to extend from the user model, so just create a separate @@ -114,10 +118,15 @@ def to_json(self): def _should_auto_move_patch(self, patch, current_date): """Determine if a patch should be automatically moved to the next commitfest. - A patch qualifies for auto-move if it both: + A patch tagged as a bug fix is always moved, so that it doesn't get + lost in a closed commitfest. Any other patch qualifies for auto-move + if it both: 1. Has had email activity within the configured number of days 2. Hasn't been failing CI for longer than the configured threshold """ + if any(tag.name in AUTO_MOVE_ALWAYS_TAG_NAMES for tag in patch.tags.all()): + return True + activity_cutoff = current_date - timedelta( days=settings.AUTO_MOVE_EMAIL_ACTIVITY_DAYS ) @@ -148,8 +157,8 @@ def _should_auto_move_patch(self, patch, current_date): def auto_move_active_patches(self): """Automatically move active patches to the next commitfest. - A patch is moved if it has recent email activity and hasn't been - failing CI for too long. + A patch is moved if it is tagged as a bug fix, or if it has recent + email activity and hasn't been failing CI for too long. """ current_date = datetime.now() @@ -170,9 +179,13 @@ def auto_move_active_patches(self): ).order_by("startdate")[0] # Get all patches with open status in this commitfest - open_pocs = self.patchoncommitfest_set.filter( - status__in=PatchOnCommitFest.OPEN_STATUSES - ).select_related("patch") + open_pocs = ( + self.patchoncommitfest_set.filter( + status__in=PatchOnCommitFest.OPEN_STATUSES + ) + .select_related("patch") + .prefetch_related("patch__tags") + ) for poc in open_pocs: if self._should_auto_move_patch(poc.patch, current_date): diff --git a/pgcommitfest/commitfest/templates/help.html b/pgcommitfest/commitfest/templates/help.html index 5807add2..609f0214 100644 --- a/pgcommitfest/commitfest/templates/help.html +++ b/pgcommitfest/commitfest/templates/help.html @@ -17,7 +17,7 @@

Commitfest

Commitfest closure

- When a Commitfest closes, patches that have been active recently are automatically moved to the next Commitfest. A patch is considered "active" if it has had email activity in the past {{auto_move_email_activity_days}} days and has not been failing CI for more than {{auto_move_max_failing_days}} days. Patches that are not automatically moved will stay in the closed Commitfest, where they will no longer be picked up by CI. Authors of such patches that have enabled "Notify on all where author" in their profile settings will receive an email notification asking them to either move the patch to the next Commitfest or close it with an appropriate status. + When a Commitfest closes, patches that have been active recently are automatically moved to the next Commitfest. A patch is considered "active" if it has had email activity in the past {{auto_move_email_activity_days}} days and has not been failing CI for more than {{auto_move_max_failing_days}} days. Patches tagged "Bugfix" are always moved, even if they are not active. Patches that are not automatically moved will stay in the closed Commitfest, where they will no longer be picked up by CI. Authors of such patches that have enabled "Notify on all where author" in their profile settings will receive an email notification asking them to either move the patch to the next Commitfest or close it with an appropriate status.

Patches

diff --git a/pgcommitfest/commitfest/tests/test_closure_notifications.py b/pgcommitfest/commitfest/tests/test_closure_notifications.py index 586079bc..c32d0d88 100644 --- a/pgcommitfest/commitfest/tests/test_closure_notifications.py +++ b/pgcommitfest/commitfest/tests/test_closure_notifications.py @@ -13,6 +13,7 @@ PatchHistory, PatchOnCommitFest, PendingNotification, + Tag, ) from pgcommitfest.mailqueue.models import QueuedMail from pgcommitfest.userprofile.models import UserProfile @@ -310,6 +311,100 @@ def test_no_auto_move_when_failing_too_long(alice, in_progress_cf, open_cf): assert "Failing Patch" in body +def test_auto_move_bugfix_without_email_activity(alice, in_progress_cf, open_cf): + """Patches tagged Bugfix should be auto-moved even without recent email activity.""" + patch = Patch.objects.create( + name="Inactive Bugfix", + lastmail=datetime.now() + - timedelta(days=settings.AUTO_MOVE_EMAIL_ACTIVITY_DAYS + 10), + ) + patch.authors.add(alice) + patch.tags.add(Tag.objects.get(name="Bugfix")) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.auto_move_active_patches() + in_progress_cf.send_closure_notifications() + + # Patch should be moved + patch.refresh_from_db() + assert patch.current_commitfest().id == open_cf.id + + # No closure email for moved patches + assert QueuedMail.objects.count() == 0 + + +def test_auto_move_bugfix_when_failing_too_long(alice, in_progress_cf, open_cf): + """Patches tagged Bugfix should be auto-moved even if CI has been failing too long.""" + patch = Patch.objects.create( + name="Failing Bugfix", + lastmail=datetime.now() + - timedelta(days=settings.AUTO_MOVE_EMAIL_ACTIVITY_DAYS + 10), + ) + patch.authors.add(alice) + patch.tags.add(Tag.objects.get(name="Bugfix")) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + CfbotBranch.objects.create( + patch=patch, + branch_id=3, + branch_name="test-branch-3", + apply_url="https://example.com", + build_url="https://example.com/build/3", + status="failed", + failing_since=datetime.now() + - timedelta(days=settings.AUTO_MOVE_MAX_FAILING_DAYS + 10), + ) + + in_progress_cf.auto_move_active_patches() + in_progress_cf.send_closure_notifications() + + # Patch should be moved + patch.refresh_from_db() + assert patch.current_commitfest().id == open_cf.id + + # No closure email for moved patches + assert QueuedMail.objects.count() == 0 + + +def test_no_auto_move_for_other_tags_without_email_activity( + alice, in_progress_cf, open_cf +): + """Tags other than Bugfix should not affect auto-move.""" + patch = Patch.objects.create( + name="Inactive Performance Patch", + lastmail=datetime.now() + - timedelta(days=settings.AUTO_MOVE_EMAIL_ACTIVITY_DAYS + 10), + ) + patch.authors.add(alice) + patch.tags.add(Tag.objects.get(name="Performance")) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.auto_move_active_patches() + in_progress_cf.send_closure_notifications() + + # Patch should NOT be moved + patch.refresh_from_db() + assert patch.current_commitfest().id == in_progress_cf.id + + # Closure email should be sent for non-moved patches + assert QueuedMail.objects.count() == 1 + + def test_auto_move_when_failing_within_threshold(alice, in_progress_cf, open_cf): """Patches failing CI within the threshold should still be auto-moved.""" patch = Patch.objects.create(