Opened 2 years ago

Closed 2 years ago

#5858 enhancement closed fixed (fixed)

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 (10)

comment:1 Changed 2 years ago by DefaultCC Plugin

  • Cc jml added

comment:2 Changed 2 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/sync-async-split-5858

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

comment:3 Changed 2 years ago by exarkun

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

refs #5858

comment:4 Changed 2 years 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

comment:5 Changed 2 years ago by Julian

  • Owner set to Julian

comment:6 Changed 2 years ago by Julian

  • Keywords review removed
  • Owner changed from Julian to exarkun

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.

comment:7 Changed 2 years ago by exarkun

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

refs #5858

comment:8 Changed 2 years 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.

comment:9 Changed 2 years ago by itamar

  • Keywords review removed
  • Owner set to exarkun

Looks good, merge if buildbot passes.

comment:10 Changed 2 years ago by exarkun

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

(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.