Opened 4 years ago

Closed 4 years ago

#6291 enhancement closed fixed (fixed)

SyncTestCase.assertNoResult should not eat any eventual result.

Reported by: Tom Prince Owned by: Tom Prince
Priority: normal Milestone:
Component: trial Keywords:
Cc: Jonathan Lange Branch: branches/assertNoResult-passthru-6291
branch-diff, diff-cov, branch-cov, buildbot
Author: tomprince

Description

Although it may be better to use multiple tests, it is often convenient to assert that a deferred hasn't got a result at one point, and then after some processing does have a specific result:

d = Deferred()
self.assertNoResult(d)
d.callback(True)
self.assertTrue(self.successResultOf(d))

Currently, assertNoResult turns any success or failure result into a successful None result, when the deferred eventually fires. successResultOf and failureResultOf suffer from the same issue, but the use case there is more tenuous.

Change History (9)

comment:1 Changed 4 years ago by DefaultCC Plugin

Cc: Jonathan Lange added

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

Here's a case to be careful of:

from twisted.trial.unittest import SynchronousTestCase
from twisted.internet.defer import Deferred
from twisted.python.failure import Failure

class Example(SynchronousTestCase):
    def test_foo(self):
        d = Deferred()
        self.assertNoResult(d)
        d.errback(Failure(Exception()))

comment:3 Changed 4 years ago by Tom Prince

And what should the result of that be? I'd assume the following test case would be successful.

from twisted.trial.unittest import SynchronousTestCase
from twisted.internet.defer import Deferred
from twisted.python.failure import Failure

class Example(SynchronousTestCase):
    def test_foo(self):
        d = Deferred()
        self.assertNoResult(d)
        d.errback(Failure(Exception("HI")))
        f = self.failureResultOf(d)
        self.assertEqual(f.value.message, "HI")

comment:4 Changed 4 years ago by Tom Prince

Which, on further consideration, suggests that at least failureResultOf (and thus successResultOf) should consume the result of the deferred.

comment:5 Changed 4 years ago by Tom Prince

Author: tomprince
Branch: branches/assertNoResult-passthru-6291

(In [37288]) Branching to assertNoResult-passthru-6291.

comment:6 Changed 4 years ago by Tom Prince

Keywords: review added

comment:7 Changed 4 years ago by therve

Keywords: review removed
Owner: set to Tom Prince
  1. The docstring of test_assertNoResult was changed in a weird way.
  1. test_assertNoResultSwallowsImmediateFailure could use asserRaises instead of a try/except.

Looks, please merge once fixed.

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

There's a typo in the news fragment.

comment:9 Changed 4 years ago by Tom Prince

Resolution: fixed
Status: newclosed

(In [37597]) Merge assertNoResult-passthru-6291: SyncTestCase.assertNoResult doesn't affect the eventual result.

Author: tom.prince Reviewers: therve Fixes: #6291

Although it may be better to use multiple tests, it is often convenient to assert that a deferred hasn't got a result at one point, and then after some processing does have a specific result.

Note: See TracTickets for help on using tickets.