Bug 69303 – watchlist: Don't add the same message to a bug more than once.

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Levin]

Right now the watchlist will run once per attachment and that may result in spammy messages.

[Adam Barth]

One approach is to look at who is CCed on the bug already.  You can also scan the comments for whatever message you would spam.

[David Levin]

(In reply to comment #1)
> One approach is to look at who is CCed on the bug already.  You can also scan the comments for whatever message you would spam.

I think we're fine on the cc list as I tried it out :) and I adding someone to the cc list who is already there seems to be a no-op.

I only care about the text but since I need to check the text (which requires fetching the bug), I might as check the cc as well to avoid making the server do unnecessary work.

[David Levin]

Created attachment 109573 [details]
Patch

[Adam Barth]

Comment on attachment 109573 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=109573&action=review

> Tools/Scripts/webkitpy/tool/mocktool.py:311
> -        return Bug(self.bug_cache.get(bug_id), self)
> +        return Bug(self.bug_cache.get(int(bug_id)), self)

Yeah, the issue of when bug ID are numbers and when they are strings is tricky.  We didn't realize it would be a problem until it was too late.  We're more careful in new objects.

> Tools/Scripts/webkitpy/tool/steps/applywatchlist.py:37
> +def is_message_in_comments(bug, message):

Module private functions start with _ by convention.  This seems like it should be a method on bug though.

> Tools/Scripts/webkitpy/tool/steps/applywatchlist.py:61
> +            messages = [message for message in messages if not is_message_in_comments(bug, message)]

You can do this more easily with the filter function.

[David Levin]

Fixed all (and added a simple unit test for the new bug method).

Committed as http://trac.webkit.org/changeset/96685