Opened 2 years ago

Closed 18 months ago

Last modified 9 months ago

#5771 enhancement closed duplicate (duplicate)

Update the test suite to not use the deprecated assertion methods

Reported by: Julian Owned by: Julian
Priority: normal Milestone:
Component: trial Keywords:
Cc: jml, Julian+Twisted@… Branch:
Author: Launchpad Bug:

Description

Following up on #4990, the test suite should use the assert* style methods.

This ticket is a preliminary requirement for merging #4991.

Attachments (1)

deprecate-fail-fix-suite.patch (428.7 KB) - added by Julian 2 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 2 years ago by DefaultCC Plugin

  • Cc jml added

Changed 2 years ago by Julian

comment:2 Changed 2 years ago by Julian

  • Cc Julian+Twisted@… added

comment:3 Changed 2 years ago by exarkun

  • Keywords review removed
  • Owner set to Julian

Thanks for working on this issue. The maximum patch length for review is about 800 lines, whereas this one comes in at over 10,000 lines. I suggest splitting the work up by sub-project (and for core, splitting it up even more than that). Then the patch can be addressed piece by piece, and no one has to try to read and accept over ten thousand lines of changes all at once. Each patch should have its own ticket.

Thanks again.

comment:4 follow-up: Changed 2 years ago by jknight

Meh.

It's only 3324 lines if you eliminate all the context lines. And, bonus, it looks easier to review that way too (seriously, not even joking; don't need context to see that self.failUnless(...) -> self.assertTrue(...) is right.)

I expect the changes to trial itself to be an issue (may want to continue using deprecated methods in tests for the deprecated methods, for example), so certainly that set of changes should be examined with more care. But the rest looks pretty darn obvious...

comment:5 in reply to: ↑ 4 Changed 2 years ago by glyph

Replying to jknight:

It's only 3324 lines if you eliminate all the context lines. And, bonus, it looks easier to review that way too (seriously, not even joking; don't need context to see that self.failUnless(...) -> self.assertTrue(...) is right.)

I expect the changes to trial itself to be an issue (may want to continue using deprecated methods in tests for the deprecated methods, for example), so certainly that set of changes should be examined with more care. But the rest looks pretty darn obvious...

Do you want to do the code review on it, then?

comment:6 Changed 18 months ago by Julian

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

This is being broken up and done in tickets #6220 - #6233.

Closing as a duplicate since there isn't really anything useful here.

comment:7 Changed 9 months ago by julian

(In [41086]) Merge remove-deprecated-test-methods-names-6222: Remove to-be-deprecated test methods from twisted.names

Author: Julian
Reviewers: tom.prince, exarkun
Fixes: #6222

Also removes the few from twisted.python. See #5771 for more info.

Note: See TracTickets for help on using tickets.