Ticket #5858 enhancement closed fixed

Opened 10 months ago

Last modified 10 months ago

Refactor twisted.trial.test.test_assertions to separate synchronous from asynchronous

Reported by: exarkun Owned by: exarkun
Priority: normal Milestone:
Component: trial Keywords:
Cc: jml Branch: branches/sync-async-split-5858
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

This test module covers a large portion of the helper methods provided by TestCase. It tests both the synchronous helpers and the asynchronous helpers (such as assertFailure and addCleanup).

To facilitate splitting up the implementation of these two categories of features, it would help to split up the tests for them as well.

Split off of #5853

Change History

1

Changed 10 months ago by DefaultCC Plugin

  • cc jml added

2

Changed 10 months ago by exarkun

  • branch set to branches/sync-async-split-5858
  • branch_author set to exarkun

(In [35167]) Branching to 'sync-async-split-5858'

3

Changed 10 months ago by exarkun

(In [35168]) Factor sync and async tests into separate TestCases; also do a little more explicit bootstrapping.

refs #5858

4

Changed 10 months ago by exarkun

  • keywords review added
  • owner exarkun deleted

Made the described changes. Note that I did a little bit of refactoring to split tests for particular assertion methods (eg failIf/assertFalse) into individual TestCase subclasses. I didn't refactor all of the tests this way though, as that would have made the patch rather large. I think that eventually this is what should be done to the tests, though. If desired I can file a ticket for that when landing this branch (and add a note to the source referring to it).

 Build results

5

Changed 10 months ago by Julian

  • owner set to Julian

6

Changed 10 months ago by Julian

  • owner changed from Julian to exarkun
  • keywords review removed

Hi :)!

I think you have a build failure on 2.6, where the context manager form of assertRaises isn't present without using unittest2. Also, some of the docstrings on TestSyncronousAssertions are missing epytext markup in their docstrings, in case that matters.

7

Changed 10 months ago by exarkun

(In [35173]) Avoid using unportable unittest.TestCase.assertRaises

refs #5858

8

Changed 10 months ago by exarkun

  • keywords review added
  • owner exarkun deleted

Thanks for the review, Julian!

I fixed the assertRaises issue in the revision linked above. I'm not too concerned about the missing epytext, as it seems to largely be in methods I'm just moving around in the file, not otherwise modifying. Updating them would be fine, but I'm not going to take that on as part of this ticket. :)

Thanks again for the review.

Latest build results  will be here.

9

Changed 10 months ago by itamar

  • owner set to exarkun
  • keywords review removed

Looks good, merge if buildbot passes.

10

Changed 10 months ago by exarkun

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

(In [35177]) Merge sync-async-split-5858

Author: exarkun Reviewer: Julian, itamar Fixes: #5858

Divide the unit tests for most of the assertion helpers on twisted.trial.unittest.TestCase into separate classes, one for synchronous helpers and another for asynchronous helpers, as a step towards supporting a synchronous-only test case base class.

Note: See TracTickets for help on using tickets.