Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#4505 task closed fixed (fixed)

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

Reported by: cyli Owned by: cyli
Priority: normal Milestone:
Component: trial Keywords: easy
Cc: jessica.mckellar@… Branch: branches/remove-trialutil-exceptions-4505
(diff, github, buildbot, log)
Author: jesstess Launchpad Bug:

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 cyli 4 years ago.

Download all attachments as: .zip

Change History (14)

Changed 4 years ago by cyli

comment:1 Changed 4 years ago by cyli

  • Keywords review added
  • Owner jml 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 4 years ago by jesstess

  • Owner set to jesstess

comment:3 Changed 4 years ago by jesstess

  • Author set to jesstess
  • Branch set to branches/remove-trialutil-exceptions-4505

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

comment:4 Changed 4 years ago by jesstess

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

refs #4505

comment:5 follow-up: Changed 4 years ago by jesstess

  • Cc jessica.mckellar@… added
  • Keywords review removed
  • Owner changed from jesstess to cyli

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

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

refs #4505

comment:7 in reply to: ↑ 5 Changed 4 years ago by cyli

  • 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 4 years ago by cyli

  • Owner cyli deleted

comment:9 Changed 4 years ago by jesstess

  • Owner set to jesstess

comment:10 Changed 4 years ago by jesstess

  • Keywords review removed
  • Owner changed from jesstess to cyli

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

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

refs #4505

comment:12 Changed 4 years ago by cyli

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

(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 4 years ago by cyli

Thanks for the quick turnaround time!

Note: See TracTickets for help on using tickets.