[Twisted-Python] Deprecating classes and writing tests
Hi, What is the best practice for deprecating a class in Twisted and writing tests for it ... and updating existing tests. I think that the part in which the deprecation code is written, is well understood and documented here http://twistedmatrix.com/documents/current/core/development/policy/compatibi... What is missing, is the part talking about how to test these changes and how to update existing tests. The compatibility / deprecation documentation include a section about how to test deprecation code, but the section is brief. http://twistedmatrix.com/documents/current/core/development/policy/compatibi... ---- This email is a follow up of a review done for this ticket https://twistedmatrix.com/trac/ticket/8368 Please also check the comments on that ticket. Please leave your feedback and we can try to apply and document it as part of the work for ticket #8368 Regards, -- Adi Roiban
We appear to have 'assertDeprecated' type methods scattered around the codebase. These should be refactored into a single location.
The thing that ends up being deprecated with the recommended technique here is the import of the class itself. So I think what's missing is the explanation that the import needs to be moved to test scope, not module scope within the tests. The tests should also be separated out and dated so it's clear what to delete when the deprecation period expires, and removals are easy. Does that answer your question sufficiently? -glyph
On 13 June 2016 at 22:46, Glyph <glyph@twistedmatrix.com> wrote:
I have created https://twistedmatrix.com/trac/ticket/8478 and I have pushed a branch to document testing deprecated module attributes. Doing test scope import will work, but I am thinking of a different method. Instead of importing/using the deprecated code as # Import raises the warning from twisted.cred.credentials import UsernameHashedPassword # Usage will not raise the warning. UsernameHashedPassword it can be imported and used in this way # Import will not raise the warning from twisted.cred import credentials # Usage will raise the warning credentials.UsernameHashedPassword I feel that the second approach is easier to integrated with self.callDeprecated If you have time, please review https://twistedmatrix.com/trac/ticket/8478 Thanks! -- Adi Roiban
We appear to have 'assertDeprecated' type methods scattered around the codebase. These should be refactored into a single location.
The thing that ends up being deprecated with the recommended technique here is the import of the class itself. So I think what's missing is the explanation that the import needs to be moved to test scope, not module scope within the tests. The tests should also be separated out and dated so it's clear what to delete when the deprecation period expires, and removals are easy. Does that answer your question sufficiently? -glyph
On 13 June 2016 at 22:46, Glyph <glyph@twistedmatrix.com> wrote:
I have created https://twistedmatrix.com/trac/ticket/8478 and I have pushed a branch to document testing deprecated module attributes. Doing test scope import will work, but I am thinking of a different method. Instead of importing/using the deprecated code as # Import raises the warning from twisted.cred.credentials import UsernameHashedPassword # Usage will not raise the warning. UsernameHashedPassword it can be imported and used in this way # Import will not raise the warning from twisted.cred import credentials # Usage will raise the warning credentials.UsernameHashedPassword I feel that the second approach is easier to integrated with self.callDeprecated If you have time, please review https://twistedmatrix.com/trac/ticket/8478 Thanks! -- Adi Roiban
participants (2)
-
Adi Roiban -
Glyph