Bug #12006

Test emails are not cleaned up in Icedove's POP test

Added by anonym 2016-12-01 00:43:04 . Updated 2017-01-24 20:42:54 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Test suite
Target version:
Start date:
2016-12-01
Due date:
% Done:

100%

Feature Branch:
test/12006-no-pop3-stray-emails
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Email Client
Deliverable for:

Description

With POP, any local deletion of a message will not affect the remove inbox. So for each run we add one message to the inbox, which will make the initial download take longer, until some later run where we reach the point where our test times out because it has to download so many messages.


Subtasks


History

#1 Updated by anonym 2016-12-11 19:46:32

  • Status changed from Confirmed to In Progress
  • Assignee changed from anonym to intrigeri
  • Target version changed from Tails_2.9.1 to Tails 2.10
  • % Done changed from 0 to 50
  • QA Check set to Ready for QA
  • Feature Branch set to test/12006-no-pop3-stray-emails

#2 Updated by intrigeri 2016-12-27 08:17:36

  • Assignee changed from intrigeri to anonym
  • QA Check changed from Ready for QA to Info Needed

Did I get it right that with this change, the “Icedove can download the inbox with POP3” scenario can’t work anymore if it’s run in isolation, and the test suite is configured to use a fresh, empty email account? IOW, it got a dependency on having already run whatever scenario includes the “I send an email to myself” step. If I got all this right: I think that’s acceptable, iff. this dependency is documented by a comment in icedove.feature (like we do e.g. in features/usb_upgrade.feature).

#3 Updated by anonym 2017-01-10 14:51:55

  • Assignee changed from anonym to intrigeri
  • QA Check changed from Info Needed to Ready for QA

The new my Icedove inbox is non-empty step simply require any email to be present in the inbox. Indeed, this requirement should be documented, so I’ve pushed a commit doing that.

#4 Updated by intrigeri 2017-01-11 08:11:14

OK, I had missed the fact that we have “Leave messages on server” enabled by default for POP3 accounts. Now, we also have:

  • “until I delete them” enabled, which kind of contradicts the justification for the way this branch deals with this problem (“We cannot clean that email up (easily) since when we use POP3 deletions won’t affect the remote inbox, only our local one”): if I got it right, deleting messages locally could actually affect the remote inbox, which would allow us to drop the requirement this branch adds;
  • “for at most 14 days”, which suggests that merely adding a message to that remote inbox when setting up the test suite is not enough: 2 weeks later, a test suite run will delete that message, and then we’re back to having a dependency on first running whatever scenario sends an email to that account.

With all this in mind, in an ideal world, I think we would still have this scenario send email to the configured account, disable “for at most 14 days”, keep “until I delete them” enabled, delete the message locally, and then do whatever is needed to make this deletion propagate to the remote server (presumably, fetching email from it again). But it’s probably overkill given we have another implementation ready, so I’ll go ahead and merge it as-is. If the limitations of this approach are a problem in practice, we can reconsider later :)

#5 Updated by intrigeri 2017-01-11 08:13:33

anonym: see previous comment (I just noticed you were not a watcher).

#6 Updated by intrigeri 2017-01-11 08:14:23

  • % Done changed from 50 to 60

Code review passes, will now test.

#7 Updated by intrigeri 2017-01-11 12:14:04

  • Status changed from In Progress to Fix committed
  • % Done changed from 60 to 100

Applied in changeset commit:3b95ba1bee6e52468d0dff54b55a23383a51cc4f.

#8 Updated by intrigeri 2017-01-11 12:23:12

  • Assignee deleted (intrigeri)
  • QA Check changed from Ready for QA to Pass

#9 Updated by anonym 2017-01-15 13:56:01

intrigeri wrote:
> But it’s probably overkill given we have another implementation ready, so I’ll go ahead and merge it as-is. If the limitations of this approach are a problem in practice, we can reconsider later :)

Perfect is the enemy of good! :)

#10 Updated by anonym 2017-01-24 20:42:54

  • Status changed from Fix committed to Resolved