Launchpad itself

Merge ~cjwatson/launchpad:fix-stormify-message into launchpad:master

  1. Git
  2. lp:~cjwatson/launchpad
  3. fix-stormify-message
  4. Merge into master
Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: cdbbf0065266f86c4c973ce243ea1e97461bc5fa
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:fix-stormify-message
Merge into: launchpad:master
Diff against target: 35 lines (+7/-3)
1 file modified
lib/lp/bugs/tests/bugs-emailinterface.rst (+7/-3)
Reviewer Review Type Date Requested Status
Ines Almeida Approve
Review via email: mp+446525@code.launchpad.net

Commit message

Fix bugs-emailinterface.rst after conversion of Message to Storm

Description of the change

The `DecoratedResultSet` used in `Bug.messages` to replace the previous `SQLRelatedJoin(prejoins=["owner"])` can't support `__contains__` without a fair bit of work. Fortunately this wasn't needed very much. There was one use in `Bug.linkMessage` which I already replaced with an explicit Storm query, but I missed the uses of it in `bugs-emailinterface.rst`.

In a test we don't need to care much about performance, so I just used `set(bug.messages)` to materialize the result of the query and turn it into something that supports `__contains__`.

To post a comment you must log in.
Revision history for this message
Ines Almeida (ines-almeida) :
review: Approve
Revision history for this message
Jürgen Gmach (jugmac00) wrote :

Would it make sense to add a comment in the tests?

It is currently not obvious when reading the tests why `set` was used. And we will probably trip over it once we convert the doctests to unit tests.

Revision history for this message
Colin Watson (cjwatson) wrote :

Well, I think you'd rather quickly find out that it's needed and put it back ... but sure, I've added a quick comment.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/bugs/tests/bugs-emailinterface.rst b/lib/lp/bugs/tests/bugs-emailinterface.rst
2index 6ae9275..1a8a900 100644
3--- a/lib/lp/bugs/tests/bugs-emailinterface.rst
4+++ b/lib/lp/bugs/tests/bugs-emailinterface.rst
5@@ -297,7 +297,11 @@ includes commands, the email has to be OpenPGP-signed.
6 >>> from lp.services.messages.interfaces.message import IMessageSet
7 >>> bug_one = bugset.get(1)
8 >>> added_message = getUtility(IMessageSet).get("<yada-yada-test1>")[0]
9- >>> added_message in bug_one.messages
10+
11+We use set() here because the DecoratedResultSet used by Bug.messages
12+doesn't currently support __contains__.
13+
14+ >>> added_message in set(bug_one.messages)
15 True
16 >>> print(bug_one.title)
17 Better summary
18@@ -522,7 +526,7 @@ to the bug:
19
20 >>> added_message = getUtility(IMessageSet).get("<yada-yada-test3>")[0]
21 >>> bug_one = bugset.get(1)
22- >>> added_message in bug_one.messages
23+ >>> added_message in set(bug_one.messages)
24 True
25
26 In these tests, every time we log in, we're fully trusted again:
27@@ -2962,7 +2966,7 @@ Attachments sent in replies to existing bugs are stored too.
28 >>> new_message = getUtility(IMessageSet).get("comment-with-attachment")[
29 ... 0
30 ... ]
31- >>> new_message in bug_one.messages
32+ >>> new_message in set(bug_one.messages)
33 True
34 >>> print_attachments(new_message.bugattachments)
35 LibraryFileAlias attachment.txt text/plain UNSPECIFIED

Subscribers

People subscribed via source and target branches

to status/vote changes: