Ticket #4467 defect closed fixed

Opened 4 years ago

Last modified 4 years ago

twisted.mail.test.test_mail.MaildirAppendStringTestCase.testAppend fails intermittently on some builders

Reported by: glyph Owned by:
Priority: normal Milestone:
Component: mail Keywords:
Cc: Branch: branches/reliable-appendMessage-4467
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

For example,  this build failed because of it.

Possibly this is related to #3812?

Change History

1

Changed 4 years ago by glyph

  • type changed from enhancement to defect

2

Changed 4 years ago by exarkun

  • branch set to branches/reliable-appendMessage-4467
  • branch_author set to exarkun

(In [29108]) Branching to 'reliable-appendMessage-4467'

3

Changed 4 years ago by exarkun

  • owner exarkun deleted
  • keywords review added

The problem is that message ordering is defined by when delivery completes, not when it starts. The test assumed that delivery would complete in the same order it was started, but on a machine with a clock of low enough precision this stopped being true.

 Build results

4

Changed 4 years ago by glyph

  • status changed from new to assigned
  • owner set to glyph

5

Changed 4 years ago by glyph

  • keywords review removed
  • status changed from assigned to new
  • owner changed from glyph to exarkun

OK, this looks good to me. Some minor stuff to fix, if you feel like it:

  1. It would be nice if appendMessage had a @return or something. It's really not clear to me when that deferred is supposed to fire, and it doesn't seem to be part of an interface.
  2. Pyflakes reports the imports from the top of the file as unused. (Actually, nevermind. Maybe just file a separate easy ticket for this one, since it seems to be over the file and unrelated to your changes.)
  3. Shouldn't _AppendTestMixin inherit from object, since it's a new class?

This looks like a big improvement though, so, merge when ready.

6

Changed 4 years ago by exarkun

(In [29148]) Expand docstring for appendMessage

refs #4467

7

Changed 4 years ago by exarkun

(In [29149]) Make _AppendTextMixin new-style

refs #4467

8

Changed 4 years ago by exarkun

  • status changed from new to closed
  • resolution set to fixed

(In [29150]) Merge reliable-appendMessage-4467

Author: exarkun Reviewer: glyph Fixes: #4467

Change the tests for MaildirMailbox.appendMessage so that they only assert a deterministic delivery-completion based delivery order. Previously they attempted to assert delivery-initiation based order, which is not guaranteed. Also remove the callLater(0, ...) in the implementation of appendMessage, as it really is not necessary.

9

Changed 3 years ago by <automation>

  • owner exarkun deleted
Note: See TracTickets for help on using tickets.