Opened 4 years ago

Closed 4 years ago

#5858 enhancement closed fixed (fixed)

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

Reported by: Jean-Paul Calderone Owned by: Jean-Paul Calderone
Priority: normal Milestone:
Component: trial Keywords:
Cc: Jonathan Lange Branch: branches/sync-async-split-5858
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

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 4 years ago by DefaultCC Plugin

Cc: Jonathan Lange added

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

Author: exarkun
Branch: branches/sync-async-split-5858

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

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

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

refs #5858

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

Keywords: review added
Owner: Jean-Paul Calderone 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 4 years ago by Julian Berman

Owner: set to Julian Berman

comment:6 Changed 4 years ago by Julian Berman

Keywords: review removed
Owner: changed from Julian Berman to Jean-Paul Calderone

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 4 years ago by Jean-Paul Calderone

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

refs #5858

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

Keywords: review added
Owner: Jean-Paul Calderone 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 4 years ago by Itamar Turner-Trauring

Keywords: review removed
Owner: set to Jean-Paul Calderone

Looks good, merge if buildbot passes.

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

Resolution: fixed
Status: newclosed

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