#5223

Committed 06 Sep 2024 06:02PM CUT coverage: 21.635% (+0.01%) from 21.622%

Build # #5223

Build Type

push

coveralls-python

Committed by

web-flow

Commit Message

Remove extraneous release field

Co-authored-by: Suhaib Mujahid <suhaibmujahid@gmail.com>

Run Details

585 of 3508 branches covered (16.68%)

0 of 1 new or added line in 1 file covered. (0.0%)

89 existing lines in 3 files now uncovered.

1940 of 8967 relevant lines covered (21.63%)

0.22 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

0.0

/bugbot/rules/inactive_reviewer.py

# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at http://mozilla.org/MPL/2.0/.

import re
from datetime import datetime
from typing import Dict, List

from dateutil.relativedelta import relativedelta
from libmozdata import utils as lmdutils
from libmozdata.connection import Connection
from libmozdata.phabricator import PhabricatorAPI
from tenacity import retry, stop_after_attempt, wait_exponential

from bugbot import utils
from bugbot.bzcleaner import BzCleaner
from bugbot.history import History
from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus

PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt")
PHAB_TABLE_PAT = re.compile(r"^\|\ \[D([0-9]+)\]\(h", flags=re.M)


class InactiveReviewer(BzCleaner):
    """Bugs with patches that are waiting for review from inactive reviewers"""

    def __init__(self, old_patch_months: int = 6):
        """Constructor

        Args:
            old_patch_months: number of months since creation of the patch to be
                considered old. If the bug has an old patch, we will mention
                abandon the patch as an option.
        """
        super(InactiveReviewer, self).__init__()
        self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"])
        self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab)
        self.ni_template = self.get_needinfo_template()
        self.old_patch_limit = (
            lmdutils.get_date_ymd("today") - relativedelta(months=old_patch_months)
        ).timestamp()

    def description(self):
        return "Bugs with inactive patch reviewers"

    def columns(self):
        return ["id", "summary", "revisions"]

    def get_bugs(self, date="today", bug_ids=[], chunk_size=None):
        bugs = super().get_bugs(date, bug_ids, chunk_size)

        rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]}
        revisions = self._get_revisions_with_inactive_reviewers(list(rev_ids))

        for bugid, bug in list(bugs.items()):
            inactive_revs = [
                revisions[rev_id] for rev_id in bug["rev_ids"] if rev_id in revisions
            ]
            if inactive_revs:
                bug["revisions"] = inactive_revs
                self._add_needinfo(bugid, inactive_revs)
            else:
                del bugs[bugid]

        # Resolving https://github.com/mozilla/bugbot/issues/1300 should clean this
        # including improve the wording in the template (i.e., "See the search query on Bugzilla").
        self.query_url = utils.get_bz_search_url({"bug_id": ",".join(bugs.keys())})

        return bugs

    def _add_needinfo(self, bugid: str, inactive_revs: list) -> None:
        ni_mails = {rev["author"]["name"] for rev in inactive_revs}
        nicknames = utils.english_list(
            sorted({rev["author"]["nick"] for rev in inactive_revs})
        )
        has_old_patch = any(
            revision["created_at"] < self.old_patch_limit for revision in inactive_revs
        )

        reviewers = {
            (reviewer["phab_username"], reviewer["status_note"])
            for revision in inactive_revs
            for reviewer in revision["reviewers"]
        }
        has_resigned = any(note == "Resigned from review" for _, note in reviewers)

        if len(reviewers) == 1:
            if has_resigned:
                summary = "a reviewer who resigned from the review"
            else:
                summary = "an inactive reviewer"
        else:
            if has_resigned:
                summary = "reviewers who are inactive or resigned from the review"
            else:
                summary = "inactive reviewers"

        comment = self.ni_template.render(
            revisions=inactive_revs,
            nicknames=nicknames,
            reviewers_status_summary=summary,
            has_old_patch=has_old_patch,
            plural=utils.plural,
            documentation=self.get_documentation(),
        )

        self.autofix_changes[bugid] = {
            "comment": {"body": comment},
            "flags": [
                {
                    "name": "needinfo",
                    "requestee": ni_mail,
                    "status": "?",
                    "new": "true",
                }
                for ni_mail in ni_mails
            ],
        }

    def _get_revisions_with_inactive_reviewers(self, rev_ids: list) -> Dict[int, dict]:
        revisions: List[dict] = []
        for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE):
            for revision in self._fetch_revisions(_rev_ids):
                if (
                    len(revision["attachments"]["reviewers"]["reviewers"]) == 0
                    or revision["fields"]["status"]["value"] != "needs-review"
                    or revision["fields"]["isDraft"]
                ):
                    continue

                reviewers = [
                    {
                        "phid": reviewer["reviewerPHID"],
                        "is_group": reviewer["reviewerPHID"].startswith("PHID-PROJ"),
                        "is_blocking": reviewer["isBlocking"],
                        "is_accepted": reviewer["status"] == "accepted",
                        "is_resigned": reviewer["status"] == "resigned",
                    }
                    for reviewer in revision["attachments"]["reviewers"]["reviewers"]
                ]

                # Group reviewers will be consider always active; so if there is
                # no other reviewers blocking, we don't need to check further.
                if any(
                    reviewer["is_group"] or reviewer["is_accepted"]
                    for reviewer in reviewers
                ) and not any(
                    not reviewer["is_accepted"]
                    for reviewer in reviewers
                    if reviewer["is_blocking"]
                ):
                    continue

                revisions.append(
                    {
                        "rev_id": revision["id"],
                        "title": revision["fields"]["title"],
                        "created_at": revision["fields"]["dateCreated"],
                        "author_phid": revision["fields"]["authorPHID"],
                        "reviewers": reviewers,
                    }
                )

        user_phids = set()
        for revision in revisions:
            user_phids.add(revision["author_phid"])
            for reviewer in revision["reviewers"]:
                user_phids.add(reviewer["phid"])

        users = self.user_activity.get_phab_users_with_status(
            list(user_phids), keep_active=True
        )

        result: Dict[int, dict] = {}
        for revision in revisions:
            # It is not useful to notify an inactive author about an inactive
            # reviewer, thus we should exclude revisions with inactive authors.
            author_info = users[revision["author_phid"]]
            if author_info["status"] != UserStatus.ACTIVE:
                continue

            revision["author"] = author_info

            inactive_reviewers = []
            for reviewer in revision["reviewers"]:
                if reviewer["is_group"]:
                    continue

                reviewer_info = users[reviewer["phid"]]
                if (
                    not reviewer["is_resigned"]
                    and reviewer_info["status"] == UserStatus.ACTIVE
                ):
                    continue

                reviewer["info"] = reviewer_info
                inactive_reviewers.append(reviewer)

            if len(inactive_reviewers) == len(revision["reviewers"]) or any(
                reviewer["is_blocking"] and not reviewer["is_accepted"]
                for reviewer in inactive_reviewers
            ):
                revision["reviewers"] = [
                    {
                        "phab_username": reviewer["info"]["phab_username"],
                        "status_note": self._get_reviewer_status_note(reviewer),
                    }
                    for reviewer in inactive_reviewers
                ]
                result[revision["rev_id"]] = revision

        return result

    @staticmethod
    def _get_reviewer_status_note(reviewer: dict) -> str:
        if reviewer["is_resigned"]:
            return "Resigned from review"

        status = reviewer["info"]["status"]
        if status == UserStatus.UNAVAILABLE:
            until = reviewer["info"]["unavailable_until"]
            if until:
                return_date = datetime.fromtimestamp(until).strftime("%b %-d, %Y")
                return f"Back {return_date}"

            return "Unavailable"

        if status == UserStatus.DISABLED:
            return "Disabled"

        return "Inactive"

    @retry(
        wait=wait_exponential(min=4),
        stop=stop_after_attempt(5),
    )
    def _fetch_revisions(self, ids: list):
        return self.phab.request(
            "differential.revision.search",
            constraints={"ids": ids},
            attachments={"reviewers": True},
        )["data"]

    def handle_bug(self, bug, data):
        rev_ids = [
            # To avoid loading the attachment content (which can be very large),
            # we extract the revision id from the file name, which is in the
            # format of "phabricator-D{revision_id}-url.txt".
            # len("phabricator-D") == 13
            # len("-url.txt") == 8
            int(attachment["file_name"][13:-8])
            for attachment in bug["attachments"]
            if attachment["content_type"] == "text/x-phabricator-request"
            and PHAB_FILE_NAME_PAT.match(attachment["file_name"])
            and not attachment["is_obsolete"]
        ]

        if not rev_ids:
            return

        # We should not comment about the same patch more than once.
        rev_ids_with_ni = set()
        for comment in bug["comments"]:
            if comment["creator"] == History.BOT and comment["raw_text"].startswith(
                "The following patch"
            ):
                rev_ids_with_ni.update(
                    int(id) for id in PHAB_TABLE_PAT.findall(comment["raw_text"])
                )

        if rev_ids_with_ni:
            rev_ids = [id for id in rev_ids if id not in rev_ids_with_ni]

        if not rev_ids:
            return

        # It will be nicer to show a sorted list of patches
        rev_ids.sort()

        bugid = str(bug["id"])
        data[bugid] = {
            "rev_ids": rev_ids,
        }
        return bug

    def get_bz_params(self, date):
        fields = [
            "comments.raw_text",
            "comments.creator",
            "attachments.file_name",
            "attachments.content_type",
            "attachments.is_obsolete",
        ]
        params = {
            "include_fields": fields,
            "resolution": "---",
            "f1": "attachments.mimetype",
            "o1": "equals",
            "v1": "text/x-phabricator-request",
        }

        return params


if __name__ == "__main__":
    InactiveReviewer().run()

1	# This Source Code Form is subject to the terms of the Mozilla Public
2	# License, v. 2.0. If a copy of the MPL was not distributed with this file,
3	# You can obtain one at http://mozilla.org/MPL/2.0/.
4
5	import re	×
6	from datetime import datetime	×
7	from typing import Dict, List	×
8
9	from dateutil.relativedelta import relativedelta	×
10	from libmozdata import utils as lmdutils	×
11	from libmozdata.connection import Connection	×
12	from libmozdata.phabricator import PhabricatorAPI	×
13	from tenacity import retry, stop_after_attempt, wait_exponential	×
14
15	from bugbot import utils	×
16	from bugbot.bzcleaner import BzCleaner	×
17	from bugbot.history import History	×
18	from bugbot.user_activity import PHAB_CHUNK_SIZE, UserActivity, UserStatus	×
19
UNCOV 20	PHAB_FILE_NAME_PAT = re.compile(r"phabricator-D([0-9]+)-url\.txt")	×
21	PHAB_TABLE_PAT = re.compile(r"^\\|\ \[D([0-9]+)\]\(h", flags=re.M)	×
22
23
UNCOV 24	class InactiveReviewer(BzCleaner):	×
25	"""Bugs with patches that are waiting for review from inactive reviewers"""
26
UNCOV 27	def __init__(self, old_patch_months: int = 6):	×
28	"""Constructor
29
30	Args:
31	old_patch_months: number of months since creation of the patch to be
32	considered old. If the bug has an old patch, we will mention
33	abandon the patch as an option.
34	"""
UNCOV 35	super(InactiveReviewer, self).__init__()	×
36	self.phab = PhabricatorAPI(utils.get_login_info()["phab_api_key"])	×
37	self.user_activity = UserActivity(include_fields=["nick"], phab=self.phab)	×
38	self.ni_template = self.get_needinfo_template()	×
39	self.old_patch_limit = (	×
40	lmdutils.get_date_ymd("today") - relativedelta(months=old_patch_months)
41	).timestamp()
42
UNCOV 43	def description(self):	×
44	return "Bugs with inactive patch reviewers"	×
45
UNCOV 46	def columns(self):	×
47	return ["id", "summary", "revisions"]	×
48
UNCOV 49	def get_bugs(self, date="today", bug_ids=[], chunk_size=None):	×
50	bugs = super().get_bugs(date, bug_ids, chunk_size)	×
51
52	rev_ids = {rev_id for bug in bugs.values() for rev_id in bug["rev_ids"]}	×
UNCOV 53	revisions = self._get_revisions_with_inactive_reviewers(list(rev_ids))	×
54
UNCOV 55	for bugid, bug in list(bugs.items()):	×
56	inactive_revs = [	×
57	revisions[rev_id] for rev_id in bug["rev_ids"] if rev_id in revisions
58	]
59	if inactive_revs:	×
60	bug["revisions"] = inactive_revs	×
UNCOV 61	self._add_needinfo(bugid, inactive_revs)	×
62	else:
63	del bugs[bugid]	×
64
65	# Resolving https://github.com/mozilla/bugbot/issues/1300 should clean this
66	# including improve the wording in the template (i.e., "See the search query on Bugzilla").
67	self.query_url = utils.get_bz_search_url({"bug_id": ",".join(bugs.keys())})	×
68
UNCOV 69	return bugs	×
70
UNCOV 71	def _add_needinfo(self, bugid: str, inactive_revs: list) -> None:	×
72	ni_mails = {rev["author"]["name"] for rev in inactive_revs}	×
UNCOV 73	nicknames = utils.english_list(	×
74	sorted({rev["author"]["nick"] for rev in inactive_revs})
75	)
76	has_old_patch = any(	×
77	revision["created_at"] < self.old_patch_limit for revision in inactive_revs
78	)
79
80	reviewers = {	×
81	(reviewer["phab_username"], reviewer["status_note"])
82	for revision in inactive_revs
83	for reviewer in revision["reviewers"]
84	}
85	has_resigned = any(note == "Resigned from review" for _, note in reviewers)	×
86
UNCOV 87	if len(reviewers) == 1:	×
UNCOV 88	if has_resigned:	×
UNCOV 89	summary = "a reviewer who resigned from the review"	×
90	else:
UNCOV 91	summary = "an inactive reviewer"	×
92	else:
UNCOV 93	if has_resigned:	×
94	summary = "reviewers who are inactive or resigned from the review"	×
95	else:
UNCOV 96	summary = "inactive reviewers"	×
97
UNCOV 98	comment = self.ni_template.render(	×
99	revisions=inactive_revs,
100	nicknames=nicknames,
101	reviewers_status_summary=summary,
102	has_old_patch=has_old_patch,
103	plural=utils.plural,
104	documentation=self.get_documentation(),
105	)
106
107	self.autofix_changes[bugid] = {	×
108	"comment": {"body": comment},
109	"flags": [
110	{
111	"name": "needinfo",
112	"requestee": ni_mail,
113	"status": "?",
114	"new": "true",
115	}
116	for ni_mail in ni_mails
117	],
118	}
119
UNCOV 120	def _get_revisions_with_inactive_reviewers(self, rev_ids: list) -> Dict[int, dict]:	×
121	revisions: List[dict] = []	×
122	for _rev_ids in Connection.chunks(rev_ids, PHAB_CHUNK_SIZE):	×
123	for revision in self._fetch_revisions(_rev_ids):	×
124	if (	×
125	len(revision["attachments"]["reviewers"]["reviewers"]) == 0
126	or revision["fields"]["status"]["value"] != "needs-review"
127	or revision["fields"]["isDraft"]
128	):
UNCOV 129	continue	×
130
131	reviewers = [	×
132	{
133	"phid": reviewer["reviewerPHID"],
134	"is_group": reviewer["reviewerPHID"].startswith("PHID-PROJ"),
135	"is_blocking": reviewer["isBlocking"],
136	"is_accepted": reviewer["status"] == "accepted",
137	"is_resigned": reviewer["status"] == "resigned",
138	}
139	for reviewer in revision["attachments"]["reviewers"]["reviewers"]
140	]
141
142	# Group reviewers will be consider always active; so if there is
143	# no other reviewers blocking, we don't need to check further.
UNCOV 144	if any(	×
145	reviewer["is_group"] or reviewer["is_accepted"]
146	for reviewer in reviewers
147	) and not any(
148	not reviewer["is_accepted"]
149	for reviewer in reviewers
150	if reviewer["is_blocking"]
151	):
152	continue	×
153
UNCOV 154	revisions.append(	×
155	{
156	"rev_id": revision["id"],
157	"title": revision["fields"]["title"],
158	"created_at": revision["fields"]["dateCreated"],
159	"author_phid": revision["fields"]["authorPHID"],
160	"reviewers": reviewers,
161	}
162	)
163
UNCOV 164	user_phids = set()	×
UNCOV 165	for revision in revisions:	×
UNCOV 166	user_phids.add(revision["author_phid"])	×
167	for reviewer in revision["reviewers"]:	×
UNCOV 168	user_phids.add(reviewer["phid"])	×
169
170	users = self.user_activity.get_phab_users_with_status(	×
171	list(user_phids), keep_active=True
172	)
173
UNCOV 174	result: Dict[int, dict] = {}	×
UNCOV 175	for revision in revisions:	×
176	# It is not useful to notify an inactive author about an inactive
177	# reviewer, thus we should exclude revisions with inactive authors.
UNCOV 178	author_info = users[revision["author_phid"]]	×
UNCOV 179	if author_info["status"] != UserStatus.ACTIVE:	×
UNCOV 180	continue	×
181
UNCOV 182	revision["author"] = author_info	×
183
UNCOV 184	inactive_reviewers = []	×
UNCOV 185	for reviewer in revision["reviewers"]:	×
UNCOV 186	if reviewer["is_group"]:	×
UNCOV 187	continue	×
188
UNCOV 189	reviewer_info = users[reviewer["phid"]]	×
UNCOV 190	if (	×
191	not reviewer["is_resigned"]
192	and reviewer_info["status"] == UserStatus.ACTIVE
193	):
UNCOV 194	continue	×
195
UNCOV 196	reviewer["info"] = reviewer_info	×
UNCOV 197	inactive_reviewers.append(reviewer)	×
198
UNCOV 199	if len(inactive_reviewers) == len(revision["reviewers"]) or any(	×
200	reviewer["is_blocking"] and not reviewer["is_accepted"]
201	for reviewer in inactive_reviewers
202	):
UNCOV 203	revision["reviewers"] = [	×
204	{
205	"phab_username": reviewer["info"]["phab_username"],
206	"status_note": self._get_reviewer_status_note(reviewer),
207	}
208	for reviewer in inactive_reviewers
209	]
UNCOV 210	result[revision["rev_id"]] = revision	×
211
UNCOV 212	return result	×
213
UNCOV 214	@staticmethod	×
UNCOV 215	def _get_reviewer_status_note(reviewer: dict) -> str:	×
UNCOV 216	if reviewer["is_resigned"]:	×
UNCOV 217	return "Resigned from review"	×
218
UNCOV 219	status = reviewer["info"]["status"]	×
UNCOV 220	if status == UserStatus.UNAVAILABLE:	×
UNCOV 221	until = reviewer["info"]["unavailable_until"]	×
UNCOV 222	if until:	×
UNCOV 223	return_date = datetime.fromtimestamp(until).strftime("%b %-d, %Y")	×
UNCOV 224	return f"Back {return_date}"	×
225
UNCOV 226	return "Unavailable"	×
227
UNCOV 228	if status == UserStatus.DISABLED:	×
UNCOV 229	return "Disabled"	×
230
UNCOV 231	return "Inactive"	×
232
UNCOV 233	@retry(	×
234	wait=wait_exponential(min=4),
235	stop=stop_after_attempt(5),
236	)
UNCOV 237	def _fetch_revisions(self, ids: list):	×
UNCOV 238	return self.phab.request(	×
239	"differential.revision.search",
240	constraints={"ids": ids},
241	attachments={"reviewers": True},
242	)["data"]
243
UNCOV 244	def handle_bug(self, bug, data):	×
UNCOV 245	rev_ids = [	×
246	# To avoid loading the attachment content (which can be very large),
247	# we extract the revision id from the file name, which is in the
248	# format of "phabricator-D{revision_id}-url.txt".
249	# len("phabricator-D") == 13
250	# len("-url.txt") == 8
251	int(attachment["file_name"][13:-8])
252	for attachment in bug["attachments"]
253	if attachment["content_type"] == "text/x-phabricator-request"
254	and PHAB_FILE_NAME_PAT.match(attachment["file_name"])
255	and not attachment["is_obsolete"]
256	]
257
UNCOV 258	if not rev_ids:	×
UNCOV 259	return	×
260
261	# We should not comment about the same patch more than once.
UNCOV 262	rev_ids_with_ni = set()	×
UNCOV 263	for comment in bug["comments"]:	×
UNCOV 264	if comment["creator"] == History.BOT and comment["raw_text"].startswith(	×
265	"The following patch"
266	):
UNCOV 267	rev_ids_with_ni.update(	×
268	int(id) for id in PHAB_TABLE_PAT.findall(comment["raw_text"])
269	)
270
UNCOV 271	if rev_ids_with_ni:	×
UNCOV 272	rev_ids = [id for id in rev_ids if id not in rev_ids_with_ni]	×
273
UNCOV 274	if not rev_ids:	×
UNCOV 275	return	×
276
277	# It will be nicer to show a sorted list of patches
UNCOV 278	rev_ids.sort()	×
279
UNCOV 280	bugid = str(bug["id"])	×
UNCOV 281	data[bugid] = {	×
282	"rev_ids": rev_ids,
283	}
UNCOV 284	return bug	×
285
UNCOV 286	def get_bz_params(self, date):	×
UNCOV 287	fields = [	×
288	"comments.raw_text",
289	"comments.creator",
290	"attachments.file_name",
291	"attachments.content_type",
292	"attachments.is_obsolete",
293	]
UNCOV 294	params = {	×
295	"include_fields": fields,
296	"resolution": "---",
297	"f1": "attachments.mimetype",
298	"o1": "equals",
299	"v1": "text/x-phabricator-request",
300	}
301
UNCOV 302	return params	×
303
304
UNCOV 305	if __name__ == "__main__":	×
UNCOV 306	InactiveReviewer().run()	×

mozilla / relman-auto-nag / #5223

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous