datatracker/ietf/review/tests_policies.py

386 lines
23 KiB
Python

# Copyright The IETF Trust 2016-2019, All Rights Reserved
import debug # pyflakes:ignore
from ietf.doc.factories import WgDraftFactory, IndividualDraftFactory
from ietf.group.factories import ReviewTeamFactory
from ietf.group.models import Group, Role
from ietf.name.models import ReviewerQueuePolicyName
from ietf.person.factories import PersonFactory
from ietf.person.fields import PersonEmailChoiceField
from ietf.person.models import Email
from ietf.review.factories import ReviewAssignmentFactory, ReviewRequestFactory
from ietf.review.models import ReviewerSettings, NextReviewerInTeam, UnavailablePeriod, ReviewWish, \
ReviewTeamSettings
from ietf.review.policies import (AssignmentOrderResolver, LeastRecentlyUsedReviewerQueuePolicy,
RotateAlphabeticallyReviewerQueuePolicy,
get_reviewer_queue_policy)
from ietf.utils.test_data import create_person
from ietf.utils.test_utils import TestCase
class GetReviewerQueuePolicyTest(TestCase):
def test_valid_policy(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"), settings__reviewer_queue_policy_id='LeastRecentlyUsed')
policy = get_reviewer_queue_policy(team)
self.assertEqual(policy.__class__, LeastRecentlyUsedReviewerQueuePolicy)
def test_missing_settings(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
ReviewTeamSettings.objects.all().delete()
with self.assertRaises(ValueError):
get_reviewer_queue_policy(team)
def test_invalid_policy_name(self):
ReviewerQueuePolicyName.objects.create(slug='invalid')
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"), settings__reviewer_queue_policy_id='invalid')
with self.assertRaises(ValueError):
get_reviewer_queue_policy(team)
class RotateAlphabeticallyReviewerAndGenericQueuePolicyTest(TestCase):
"""
These tests also cover the common behaviour in AbstractReviewerQueuePolicy,
as that's difficult to test on it's own.
"""
def test_default_reviewer_rotation_list(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
reviewers = [
create_person(team, "reviewer", name="Test Reviewer{}".format(i), username="testreviewer{}".format(i))
for i in range(5)
]
# This reviewer should never be included.
unavailable_reviewer = create_person(team, "reviewer", name="unavailable reviewer", username="unavailablereviewer")
UnavailablePeriod.objects.create(
team=team,
person=unavailable_reviewer,
start_date='2000-01-01',
availability='unavailable',
)
# This should not have any impact. Canfinish unavailable reviewers are included in
# the default rotation, and filtered further when making assignment choices.
UnavailablePeriod.objects.create(
team=team,
person=reviewers[1],
start_date='2000-01-01',
availability='canfinish',
)
# Default policy without a NextReviewerInTeam
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, reviewers)
# Policy with a current NextReviewerInTeam
NextReviewerInTeam.objects.create(team=team, next_reviewer=reviewers[3])
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, reviewers[3:] + reviewers[:3])
# Policy with a NextReviewerInTeam that has left the team.
Role.objects.get(person=reviewers[1]).delete()
NextReviewerInTeam.objects.filter(team=team).update(next_reviewer=reviewers[1])
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, reviewers[2:] + reviewers[:1])
def test_setup_reviewer_field(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
reviewer_0 = create_person(team, "reviewer", name="Test Reviewer-0", username="testreviewer0")
reviewer_1 = create_person(team, "reviewer", name="Test Reviewer-1", username="testreviewer1")
reviewer_2 = create_person(team, "reviewer", name="Test Reviewer-2", username="testreviewer2")
reviewer_3 = create_person(team, "reviewer", name="Test Reviewer-3", username="testreviewer3")
review_req = ReviewRequestFactory(team=team, type_id='early')
review_req_2 = ReviewRequestFactory(team=team, type_id='early', doc=review_req.doc)
review_req_3 = ReviewRequestFactory(team=team, type_id='early', doc=review_req.doc)
ReviewAssignmentFactory(review_request=review_req, reviewer=reviewer_1.email(), state_id='part-completed')
ReviewAssignmentFactory(review_request=review_req_2, reviewer=reviewer_2.email(), state_id='rejected')
ReviewAssignmentFactory(review_request=review_req_3, reviewer=reviewer_3.email(), state_id='no-response')
field = PersonEmailChoiceField(label="Assign Reviewer", empty_label="(None)", required=False)
policy.setup_reviewer_field(field, review_req)
addresses = list( map( lambda choice: choice[0], field.choices ) )
self.assertNotIn(
str(reviewer_2.email()), addresses,
"Reviews should not suggest people who have rejected this request in the past")
self.assertNotIn(
str(reviewer_3.email()), addresses,
"Reviews should not suggest people who have not responded to this request in the past.")
self.assertEqual(field.choices[0], ('', '(None)'))
self.assertEqual(field.choices[1][0], str(reviewer_0.email()))
self.assertEqual(field.choices[2][0], str(reviewer_1.email()))
self.assertEqual(field.choices[1][1], 'Test Reviewer-0: #1')
self.assertEqual(field.choices[2][1], 'Test Reviewer-1: #2; 1 partially complete')
self.assertEqual(field.initial, str(reviewer_1.email()))
def test_recommended_assignment_order(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
reviewer_high = create_person(team, "reviewer", name="Test Reviewer-1-high", username="testreviewerhigh")
reviewer_low = create_person(team, "reviewer", name="Test Reviewer-0-low", username="testreviewerlow")
# reviewer_high appears later in the default rotation, but reviewer_low is the author
doc = WgDraftFactory(group__acronym='mars', rev='01', authors=[reviewer_low])
review_req = ReviewRequestFactory(doc=doc, team=team, type_id='early')
order = policy.recommended_assignment_order(Email.objects.all(), review_req)
self.assertEqual(order[0][0], str(reviewer_high.email()))
self.assertEqual(order[1][0], str(reviewer_low.email()))
self.assertEqual(order[0][1], 'Test Reviewer-1-high: #2')
self.assertEqual(order[1][1], 'Test Reviewer-0-low: is author of document; #1')
with self.assertRaises(ValueError):
review_req_other_team = ReviewRequestFactory(doc=doc, type_id='early')
policy.recommended_assignment_order(Email.objects.all(), review_req_other_team)
def test_update_policy_state_for_assignment(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
# make a bunch of reviewers
reviewers = [
create_person(team, "reviewer", name="Test Reviewer{}".format(i), username="testreviewer{}".format(i))
for i in range(5)
]
self.assertEqual(reviewers, policy.default_reviewer_rotation_list())
def reviewer_settings_for(person):
return (ReviewerSettings.objects.filter(team=team, person=person).first()
or ReviewerSettings(team=team, person=person))
def get_skip_next(person):
return reviewer_settings_for(person).skip_next
# Regular in-order assignment without skips
reviewer0_settings = reviewer_settings_for(reviewers[0])
reviewer0_settings.request_assignment_next = True
reviewer0_settings.save()
policy.update_policy_state_for_assignment(assignee_person=reviewers[0], add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[1])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 0)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# request_assignment_next should be reset after any assignment
self.assertFalse(reviewer_settings_for(reviewers[0]).request_assignment_next)
# In-order assignment with add_skip
policy.update_policy_state_for_assignment(assignee_person=reviewers[1], add_skip=True)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1) # from current add_skip=True
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# In-order assignment to 2, but 3 has a skip_next, so 4 should be assigned.
# 3 has skip_next decreased as it is skipped over, 1 retains its skip_next
reviewer3_settings = reviewer_settings_for(reviewers[3])
reviewer3_settings.skip_next = 2
reviewer3_settings.save()
policy.update_policy_state_for_assignment(assignee_person=reviewers[2], add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1) # from previous add_skip=true
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1) # from manually set skip_next - 1
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Out of order assignments, nothing should change,
# except the add_skip=True should still apply
policy.update_policy_state_for_assignment(assignee_person=reviewers[3], add_skip=False)
policy.update_policy_state_for_assignment(assignee_person=reviewers[2], add_skip=False)
policy.update_policy_state_for_assignment(assignee_person=reviewers[1], add_skip=False)
policy.update_policy_state_for_assignment(assignee_person=reviewers[0], add_skip=True)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4])
self.assertEqual(get_skip_next(reviewers[0]), 1) # from current add_skip=True
self.assertEqual(get_skip_next(reviewers[1]), 1)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Regular assignment, testing wrap-around
policy.update_policy_state_for_assignment(assignee_person=reviewers[4], add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0) # skipped over with this assignment
self.assertEqual(get_skip_next(reviewers[1]), 0) # skipped over with this assignment
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Leave only a single reviewer remaining, which should not trigger an infinite loop.
# The deletion also causes NextReviewerInTeam to be deleted.
[reviewer.delete() for reviewer in reviewers[1:]]
self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list())
policy.update_policy_state_for_assignment(assignee_person=reviewers[0], add_skip=False)
# No NextReviewerInTeam should be created, the only possible next is the excluded assignee.
self.assertFalse(NextReviewerInTeam.objects.filter(team=team))
self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list())
def test_return_reviewer_to_rotation_top(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team",
list_email="rotationteam@ietf.org",
parent=Group.objects.get(acronym="farfut"))
reviewer = create_person(team, "reviewer", name="reviewer", username="reviewer")
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
policy.return_reviewer_to_rotation_top(reviewer)
self.assertTrue(ReviewerSettings.objects.get(person=reviewer).request_assignment_next)
class LeastRecentlyUsedReviewerQueuePolicyTest(TestCase):
"""
These tests only cover where this policy deviates from
RotateAlphabeticallyReviewerQueuePolicy - the common behaviour
inherited from AbstractReviewerQueuePolicy is covered in
RotateAlphabeticallyReviewerQueuePolicyTest.
"""
def test_default_reviewer_rotation_list(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team",
list_email="rotationteam@ietf.org",
parent=Group.objects.get(acronym="farfut"))
policy = LeastRecentlyUsedReviewerQueuePolicy(team)
reviewers = [
create_person(team, "reviewer", name="Test Reviewer{}".format(i),
username="testreviewer{}".format(i))
for i in range(5)
]
# This reviewer should never be included.
unavailable_reviewer = create_person(team, "reviewer", name="unavailable reviewer",
username="unavailablereviewer")
UnavailablePeriod.objects.create(
team=team,
person=unavailable_reviewer,
start_date='2000-01-01',
availability='unavailable',
)
# This should not have any impact. Canfinish unavailable reviewers are included in
# the default rotation, and filtered further when making assignment choices.
UnavailablePeriod.objects.create(
team=team,
person=reviewers[1],
start_date='2000-01-01',
availability='canfinish',
)
# This reviewer has an assignment, but is no longer in the team and should not be in rotation.
out_of_team_reviewer = PersonFactory()
ReviewAssignmentFactory(review_request__team=team, reviewer=out_of_team_reviewer.email())
# No known assignments, order in PK order.
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, reviewers)
# Regular accepted assignments. Note that one is older and one is newer than reviewer 0's,
# the newest one should count for ordering, i.e. reviewer 1 should be later in the list.
ReviewAssignmentFactory(reviewer=reviewers[1].email(), assigned_on='2019-01-01',
state_id='accepted', review_request__team=team)
ReviewAssignmentFactory(reviewer=reviewers[1].email(), assigned_on='2017-01-01',
state_id='accepted', review_request__team=team)
# Rejected assignment, should not affect reviewer 2's position
ReviewAssignmentFactory(reviewer=reviewers[2].email(), assigned_on='2020-01-01',
state_id='rejected', review_request__team=team)
# Completed assignments, assigned before the most recent assignment of reviewer 1,
# but completed after (assign date should count).
ReviewAssignmentFactory(reviewer=reviewers[0].email(), assigned_on='2018-01-01',
completed_on='2020-01-01', state_id='completed',
review_request__team=team)
ReviewAssignmentFactory(reviewer=reviewers[0].email(), assigned_on='2018-05-01',
completed_on='2020-01-01', state_id='completed',
review_request__team=team)
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, [reviewers[2], reviewers[3], reviewers[4], reviewers[0], reviewers[1]])
def test_return_reviewer_to_rotation_top(self):
# Should do nothing, this is implicit in this policy, no state change is needed.
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team",
list_email="rotationteam@ietf.org",
parent=Group.objects.get(acronym="farfut"))
reviewer = create_person(team, "reviewer", name="reviewer", username="reviewer")
policy = LeastRecentlyUsedReviewerQueuePolicy(team)
policy.return_reviewer_to_rotation_top(reviewer)
class AssignmentOrderResolverTests(TestCase):
def test_determine_ranking(self):
# reviewer_high is second in the default rotation, reviewer_low is first
# however, reviewer_high hits every score increase, reviewer_low hits every score decrease
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
reviewer_high = create_person(team, "reviewer", name="Test Reviewer-high", username="testreviewerhigh")
reviewer_low = create_person(team, "reviewer", name="Test Reviewer-low", username="testreviewerlow")
reviewer_unavailable = create_person(team, "reviewer", name="Test Reviewer-unavailable", username="testreviewerunavailable")
# This reviewer should be entirely ignored because it is not in the rotation list.
create_person(team, "reviewer", name="Test Reviewer-out-of-rotation", username="testreviewer-out-of-rotation")
# Create a document with ancestors, that also triggers author check, AD check and group check
doc_individual = IndividualDraftFactory()
doc_wg = WgDraftFactory(relations=[('replaces', doc_individual)])
doc_middle_wg = WgDraftFactory(relations=[('replaces', doc_wg)])
doc = WgDraftFactory(group__acronym='mars', rev='01', authors=[reviewer_low], ad=reviewer_low, shepherd=reviewer_low.email(), relations=[('replaces', doc_middle_wg)])
Role.objects.create(group=doc.group, person=reviewer_low, email=reviewer_low.email(), name_id='advisor')
# Trigger previous review check (including finding ancestor documents) and completed review stats.
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc_individual, reviewer=reviewer_high.email(), state_id='completed')
# Trigger other review stats
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_high.email(), state_id='no-response')
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_high.email(), state_id='part-completed')
# Trigger review wish check
ReviewWish.objects.create(team=team, doc=doc, person=reviewer_high)
# This period should not have an impact, because it is the canfinish type,
# and this reviewer has reviewed previously.
UnavailablePeriod.objects.create(
team=team,
person=reviewer_high,
start_date='2000-01-01',
availability='canfinish',
)
# This period should exclude this reviewer entirely, as it is 'canfinish',
# but this reviewer has not reviewed previously.
UnavailablePeriod.objects.create(
team=team,
person=reviewer_unavailable,
start_date='2000-01-01',
availability='canfinish',
)
# Trigger "reviewer has rejected before"
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_low.email(), state_id='rejected')
# Trigger max frequency and open review stats
ReviewAssignmentFactory(review_request__team=team, reviewer=reviewer_low.email(), state_id='assigned', review_request__doc__pages=10)
# Trigger skip_next, max frequency, filter_re
ReviewerSettings.objects.create(
team=team,
person=reviewer_low,
filter_re='.*draft.*',
skip_next=2,
min_interval=91,
)
# Trigger "assign me next"
ReviewerSettings.objects.create(
team=team,
person=reviewer_high,
request_assignment_next=True,
)
review_req = ReviewRequestFactory(doc=doc, team=team, type_id='early')
rotation_list = [reviewer_low, reviewer_high, reviewer_unavailable]
order = AssignmentOrderResolver(Email.objects.all(), review_req, rotation_list)
ranking = order.determine_ranking()
self.assertEqual(len(ranking), 2)
self.assertEqual(ranking[0]['email'], reviewer_high.email())
self.assertEqual(ranking[1]['email'], reviewer_low.email())
# These scores follow the ordering of https://trac.tools.ietf.org/tools/ietfdb/wiki/ReviewerQueuePolicy,
self.assertEqual(ranking[0]['scores'], [ 1, 1, 1, 1, 1, 1, 0, 0, -1])
self.assertEqual(ranking[1]['scores'], [-1, -1, -1, -1, -1, -1, -91, -2, 0])
self.assertEqual(ranking[0]['label'], 'Test Reviewer-high: unavailable indefinitely (Can do follow-ups); requested to be selected next for assignment; reviewed document before; wishes to review document; #2; 1 no response, 1 partially complete, 1 fully completed')
self.assertEqual(ranking[1]['label'], 'Test Reviewer-low: rejected review of document before; is author of document; filter regexp matches; max frequency exceeded, ready in 91 days; skip next 2; #1; currently 1 open, 10 pages')