#6105 enhancement closed fixed (fixed)

Implement assertNotFired, assertFired and assertErrbackFired on SynchronousTestCase

Reported by: itamar Owned by: exarkun
Priority: normal Milestone:
Component: trial Keywords:
Cc: mithrandi@… Branch: branches/sync-deferred-testing-helper-6105-2
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

In many tests it is useful to deal with Deferreds in a synchronous manner. The current idiom is:

result = []
d.addCallback(result.append)
self.assertTrue(result)

This is icky (a technical term!). We should instead have assertions on twisted.trial.unittest.SynchronousTestCase that can run against Deferreds without relying on the reactor (i.e. no need to return a Deferred from the test).

Attachments (1)

stuff.py (1.7 KB) - added by mithrandi 22 months ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 22 months ago by itamar

Additionally the trial howto should be fixed: currently it tells you to return a Deferred even for a test that doesn't use the reactor. Using these APIs in the documentation would be better.

Changed 22 months ago by mithrandi

comment:2 Changed 22 months ago by mithrandi

Attaching some stuff I hacked up, although it's untested, and exarkun says there's already a few implementations of this in Twisted.

comment:3 Changed 22 months ago by mithrandi

  • Cc mithrandi@… added

comment:4 Changed 21 months ago by exarkun

  • Component changed from core to trial
  • Keywords trial removed
  • Owner set to exarkun

comment:5 Changed 21 months ago by exarkun

  • Author set to exarkun
  • Branch set to branches/sync-deferred-testing-helper-6105

(In [36454]) Branching to 'sync-deferred-testing-helper-6105'

comment:6 Changed 21 months ago by exarkun

(In [36455]) SynchronousTestCase.resultOf

refs #6105

comment:7 Changed 21 months ago by exarkun

(In [36457]) Switch to resultOf

refs #6105

comment:8 Changed 21 months ago by exarkun

Once I get back to this, I think it's going to be successResultOf, failureResultOf, and assertNotFired.

comment:9 Changed 21 months ago by exarkun

  • Branch changed from branches/sync-deferred-testing-helper-6105 to branches/sync-deferred-testing-helper-6105-2

(In [36477]) Branching to 'sync-deferred-testing-helper-6105-2'

comment:10 Changed 21 months ago by exarkun

  • Keywords review added
  • Owner exarkun deleted

comment:11 Changed 21 months ago by therve

  • Keywords review removed
  • Owner set to exarkun
  1. Pyflakes:

twisted/web/test/test_agent.py:18: 'reactor' imported but unused
twisted/web/test/test_agent.py:18: 'interfaces' imported but unused
twisted/web/test/test_agent.py:593: redefinition of unused 'error' from line 15

2.

+        if len(result) == 0:
...
+        if len(result) == 1:

It's a stylistic point but I generally prefer to do 'if not result' 'if result' in those cases where you know it's a list. I guess our coding standard doesn't make everything explicit. Your call, etc.

  1. I don't think you can put a L{} in @raise, epytext seems to be complaining about it. There are also lots of invalid ref in test_assertions.

4.

+<h2>Testing Deferreds Without The Reactor</h2>

No capitalization? :)

  1. The howto file can't be parsed properly, there is missing </em>
  1. The 2 elif clauses in successResultOf and failureResultOf don't seem to be tested.

Thanks!

comment:12 Changed 21 months ago by exarkun

  • Keywords review added
  • Owner exarkun deleted

Thanks!

Pyflakes ...

Fixed in r36487 and r36491

It's a stylistic point but I generally prefer to do 'if not result' 'if result' in those cases where you know it's a list

Hummm. Okay. Another stylistic point, I generally prefer avoiding 'if not x' when I can somehow write 'if x'. I don't see a good way to rewrite (2 out of 3 of) these as 'if result' instead of 'if not result'. However, 'if not result' doesn't seem too bad in those two cases. Fixed in r36493.

I don't think you can put a L{} in @raise

I always mess that one up. Fixed in r36488.

No capitalization? :)

Fixed in r36489.

The howto file can't be parsed properly

Fixed in r36490.

The 2 elif clauses in successResultOf and failureResultOf don't seem to be tested

Embarrassing. Fixed in r36492.

comment:13 Changed 21 months ago by therve

  • Keywords review removed
  • Owner set to exarkun

7.

+    failure  = Failure(Exception("Bad times"))

There is an extra space before "=".

  1. There are many new invalid ref in epytext. At least for the public methods:
    twisted.trial._synctest._Assertions.successResultOf:594 invalid ref to Deferred
    twisted.trial._synctest._Assertions.successResultOf:594 invalid ref to Deferred.callback
    twisted.trial._synctest._Assertions.successResultOf:594 invalid ref to Deferred.errback
    twisted.trial._synctest._Assertions.successResultOf:594 invalid ref to Deferred
    twisted.trial._synctest._Assertions.successResultOf:594 invalid ref to Deferred
    twisted.trial._synctest._Assertions.failureResultOf:624 invalid ref to Deferred
    twisted.trial._synctest._Assertions.failureResultOf:624 invalid ref to Deferred.callback
    twisted.trial._synctest._Assertions.failureResultOf:624 invalid ref to Deferred.errback
    twisted.trial._synctest._Assertions.failureResultOf:624 invalid ref to Deferred
    twisted.trial._synctest._Assertions.failureResultOf:624 invalid ref to Deferred
    twisted.trial._synctest._Assertions.assertNoResult:657 invalid ref to Deferred
    twisted.trial._synctest._Assertions.assertNoResult:657 invalid ref to Deferred.callback
    twisted.trial._synctest._Assertions.assertNoResult:657 invalid ref to Deferred.errback
    twisted.trial._synctest._Assertions.assertNoResult:657 invalid ref to Deferred
    twisted.trial._synctest._Assertions.assertNoResult:657 invalid ref to Deferred
    twisted.trial._synctest._Assertions.assertNoResult:657 invalid ref to Deferred
    twisted.trial._synctest._Assertions.assertNoResult:657 invalid ref to Deferred
    

Please merge once fixed.

comment:14 Changed 21 months ago by exarkun

(In [36502]) Fix epytext link errors

refs #6105

comment:15 Changed 21 months ago by exarkun

(In [36503]) remove extra whitespace

refs #6105

comment:16 Changed 21 months ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(In [36506]) Merge sync-deferred-testing-helper-6105-2

Author: exarkun
Reviewer: therve
Fixes: #6105

Add successResultOf, failureResultOf, and assertNoResult to SynchronousTestCase, to help
developers write tests for Deferred-using code.

Note: See TracTickets for help on using tickets.