Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#4505 task closed fixed (fixed)

Deprecate or remove unused warnings and exceptions in twisted.trial.util

Reported by: Ying Li Owned by: Ying Li
Priority: normal Milestone:
Component: trial Keywords: easy
Cc: jesstess Branch: branches/remove-trialutil-exceptions-4505
branch-diff, diff-cov, branch-cov, buildbot
Author: jesstess

Description

FailureError, DirtyReactorWarning, DirtyReactorError, PendingTimedCallsError have been deprecated since Twisted 8.0, although the deprecation was stated in a docstring rather than as a warning.

This should either be deprecated correctly (via warnings) or removed. Removing them does not seem to affect any tests.

Attachments (1)

4505.diff (2.1 KB) - added by Ying Li 7 years ago.

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by Ying Li

Attachment: 4505.diff added

comment:1 Changed 7 years ago by Ying Li

Keywords: review added
Owner: Jonathan Lange deleted

I am in favor of removal, as it is simpler. Also, the docstring for twisted.trial.util say that it is a module for internal use only.

comment:2 Changed 7 years ago by jesstess

Owner: set to jesstess

comment:3 Changed 7 years ago by jesstess

Author: jesstess
Branch: branches/remove-trialutil-exceptions-4505

(In [29309]) Branching to 'remove-trialutil-exceptions-4505'

comment:4 Changed 7 years ago by jesstess

(In [29310]) Apply patch 4505.diff by cyli.

refs #4505

comment:5 Changed 7 years ago by jesstess

Cc: jesstess added
Keywords: review removed
Owner: changed from jesstess to Ying Li

Thanks for tackling all these deprecations and removals, cyli!

Given the "This code is for Trial's internal use. Do NOT use this code if you are writing tests. It is subject to change at the Trial maintainer's whim." disclaimer at the top of util.py, I agree with removal instead of deprecation.

Some feedback:

  • In the topfile, the message is duplicated.
  • In util.py, pyflakes catches that _ _all_ _ still references the removed Exceptions.
  • There are still references to DirtyReactorError in twisted/trial/test/test_reporter.py and twisted/trial/reporter.py

comment:6 Changed 7 years ago by Ying Li

(In [29313]) Bump copyright (previous commit: fixes for review)

refs #4505

comment:7 in reply to:  5 Changed 7 years ago by Ying Li

Keywords: review added
  • In the topfile, the message is duplicated.

Huh, odd, it wasn't in the diff. Fixed though.

  • In util.py, pyflakes catches that _ _all_ _ still references the removed Exceptions.

Removed.

  • There are still references to DirtyReactorError in twisted/trial/test/test_reporter.py and twisted/trial/reporter.py

Looks like someone forgot to modify the comments when they changed over from using DirtyReactorError to using DirtyReactorAggregateError. Fixed.

comment:8 Changed 7 years ago by Ying Li

Owner: Ying Li deleted

comment:9 Changed 7 years ago by jesstess

Owner: set to jesstess

comment:10 Changed 7 years ago by jesstess

Keywords: review removed
Owner: changed from jesstess to Ying Li

C{DirtyReactorAggregateError} might as well be L{DirtyReactorAggregateError} in test_warningsEnabled and test_warningsMaskErrors. Other than that, I think this good to merge.

comment:11 Changed 7 years ago by Ying Li

(In [29316]) Updated C{DirtyReactorAggregateError} to L{DirtyReactorAggregateError}

refs #4505

comment:12 Changed 7 years ago by Ying Li

Resolution: fixed
Status: newclosed

(In [29317]) Merge remove-trialutil-exceptions-4505 : remove deprecated exceptions in t.trial.util

Author: cyli Reviewer: jesstess Fixes: #4505

Remove deprecated warnings and exceptions from t.trial.util, as well as some comments in t.trial.reporter and t.trial.test.test_reporter

comment:13 Changed 7 years ago by Ying Li

Thanks for the quick turnaround time!

Note: See TracTickets for help on using tickets.