Opened 16 years ago

Closed 15 years ago

#2266 defect closed fixed (fixed)

twisted.mail.test.test_mail.ProcessAliasTestCase leaves the reactor unclean

Reported by: Jonathan Lange Owned by:
Priority: highest Milestone:
Component: mail Keywords: tests
Cc: therve Branch:
Author:

Description


Change History (13)

comment:1 Changed 15 years ago by therve

I think this has been solved in [19640]. Do you confirm ?

comment:2 Changed 15 years ago by therve

Cc: therve added

comment:3 Changed 15 years ago by therve

Forget it, it seems it's an intermittent problem.

comment:4 Changed 15 years ago by therve

Owner: changed from Jonathan Lange to therve

comment:5 Changed 15 years ago by therve

(In [21458]) Use a MockProcessAlias that actually doesn't launch a process.

Refs #2266

comment:6 Changed 15 years ago by therve

Keywords: review added
Owner: therve deleted
Priority: normalhighest

Ready to review in mail-alias-unclean-2266.

comment:7 Changed 15 years ago by therve

Merged forward in mail-alias-unclean-2266-2.

comment:8 Changed 15 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to therve
  • test_processAlias misspells transferred.
  • I'd say something more specific in the docstring for test_aliasResolution, maybe mention the types of aliases and what each corresponds to.
  • twisted.mail.alias imports twisted.mail.smtp imports twisted.internet.reactor at module scope, so there's no reason for twisted.mail.alias.ProcessAlias.spawnProcess to nest the reactor import instead of having it at module scope

Hmmm. This gets rid of the unclean errors from this test. It no longer exercises the process spawning code at all though. It wasn't really testing that behavior before though. Probably there should be a ticket for adding test coverage for ProcessAlias.

comment:9 Changed 15 years ago by therve

(In [21526]) Process review comments

Refs #2266

comment:10 Changed 15 years ago by therve

Keywords: review added
Owner: therve deleted

Thanks for the comments, the docstring of test_aliasResolution was a bit hard to produce, I hope it will be fine enough.

Note that test_processAlias does spawn a real process, and check its result. But I think file wrapper are untested.

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

Keywords: review removed
Owner: set to therve

Cool, merge please :)

comment:12 Changed 15 years ago by therve

Resolution: fixed
Status: newclosed

(In [21528]) Merge mail-alias-unclean-2266-2

Author: therve Reviewer: exarkun Fixes #2266

Remove unclean errors from twisted.mail.test.test_mail.ProcessAliasTestCase.

comment:13 Changed 11 years ago by <automation>

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