Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions pgcommitfest/commitfest/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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()

Expand All @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion pgcommitfest/commitfest/templates/help.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ <h2>Commitfest</h2>

<h3>Commitfest closure</h3>
<p>
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.
</p>

<h2>Patches</h2>
Expand Down
95 changes: 95 additions & 0 deletions pgcommitfest/commitfest/tests/test_closure_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
PatchHistory,
PatchOnCommitFest,
PendingNotification,
Tag,
)
from pgcommitfest.mailqueue.models import QueuedMail
from pgcommitfest.userprofile.models import UserProfile
Expand Down Expand Up @@ -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(
Expand Down