Opened 11 years ago

Closed 11 years ago

#2832 defect closed fixed (fixed)

twisted.mail.test.test_smtp.TimeoutTestCase.testSMTPClient fails intermittently due to timing issues

Reported by: Jean-Paul Calderone Owned by:
Priority: highest Milestone:
Component: mail Keywords:
Cc: therve Branch:
Author:

Description

The tests in TimeoutTestCase in this module use the system clock, which means they cannot be deterministic. They should use a deterministic clock instead.

Attachments (1)

2832.diff (2.9 KB) - added by therve 11 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 11 years ago by therve

Owner: changed from Jean-Paul Calderone to therve

Changed 11 years ago by therve

Attachment: 2832.diff added

comment:2 Changed 11 years ago by therve

(In [21533]) Remove time related calls.

Refs #2832

comment:3 Changed 11 years ago by therve

Cc: therve added
Keywords: review added
Owner: therve deleted
Priority: normalhighest

Ready to review in test-smtp-timeout-2832.

comment:4 Changed 11 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to therve

That twisted.internet import line in test_smtp is getting kinda long. :)

Maybe the assertion about length of calls being 1 would be better in a callback on d, instead of having list.append as the callback.

comment:5 Changed 11 years ago by therve

I don't understand. Do you mean removing the assertion? Because I don't see how it can be done in the callback, since it's asserting that the callback has been called? Maybe I can check the clock time in the callback?

comment:6 Changed 11 years ago by therve

Keywords: review added
Owner: changed from therve to Jean-Paul Calderone

comment:7 Changed 11 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: changed from Jean-Paul Calderone to therve

Sorry, yea, I phrased that poorly. Instead of using calls.append as a callback, define a function which makes an assertion about clock.seconds() and use that as the callback (as you said).

comment:8 Changed 11 years ago by therve

(In [21627]) Process review comments.

Refs #2832

comment:9 Changed 11 years ago by therve

(In [21631]) Return deferreds from assertFailure

Refs #2832

comment:10 Changed 11 years ago by therve

Sorry the last commit is irrelevant here.

comment:11 Changed 11 years ago by therve

(In [21640]) Some docs and comments.

Refs #2832

comment:12 Changed 11 years ago by therve

Keywords: review added
Owner: therve deleted

comment:13 Changed 11 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to therve

Looks good

comment:14 Changed 11 years ago by therve

Resolution: fixed
Status: newclosed

(In [21666]) Merge test-smtp-timeout-2832

Author: therve Reviewer: exarkun Fixes #2832

Use a deterministic clock in twisted.mail.test.test_smtp.TimeoutTestCase to avoid intermittent failures.

comment:15 Changed 7 years ago by <automation>

Owner: therve deleted
Note: See TracTickets for help on using tickets.