New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#8478 Document testing deprecated modules. #11971
base: trunk
Are you sure you want to change the base?
Changes from 5 commits
a0ccef7
c05db34
52ceab6
6a44cd7
612d11d
4fb3c0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -445,12 +445,33 @@ Modules cannot have properties, so module attributes should be deprecated using | |
Modules | ||
^^^^^^^ | ||
|
||
To deprecate an entire module, :py:func:`deprecatedModuleAttribute <twisted.python.deprecate.deprecatedModuleAttribute>` can be used on the parent package's ``__init__.py``. | ||
To deprecate an entire module there are two options: | ||
|
||
There are two other options: | ||
* Put a `warnings.warn()` call into the top-level code of the module, with `stacklevel=3`. | ||
* Deprecate all of the attributes of the module and sub-modules using :py:func:`deprecatedModuleAttribute <twisted.python.deprecate.deprecatedModuleAttribute>`. | ||
|
||
* Put a warnings.warn() call into the top-level code of the module. | ||
* Deprecate all of the attributes of the module. | ||
Using a single `deprecatedModuleAttribute` on the parent module will not work. | ||
For example if we have `twisted.old_code.sub.module.SomeClass` and you add the deprecation to `twisted` for the name `old_code`, | ||
a warning is raised for `from twisted import old_code`, | ||
but is not raised from `from twisted.old_code.sub.module import SomeClass`. | ||
|
||
This is why it's recommended to just use `warnings.warn()` at the top level of the module. | ||
|
||
.. code-block:: python | ||
|
||
""" | ||
The normal docstring of the top-level package. | ||
|
||
This package was deprecated in Twisted NEXT | ||
""" | ||
import warnings | ||
Comment on lines
+462
to
+467
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This module should have an introduction that explains what the code sample is meant to illustrate before launching into it. |
||
|
||
from incremental import Version, getVersionString | ||
|
||
warningString = "twisted.old_code was deprecated at {}".format( | ||
getVersionString(Version("Twisted", "NEXT", 0, 0)) | ||
) | ||
warnings.warn(warningString, DeprecationWarning, stacklevel=3) | ||
|
||
|
||
Testing Deprecation Code | ||
|
@@ -551,6 +572,20 @@ Making calls to the deprecated code without raising these warnings can be done u | |
|
||
self.assertEqual('some-value', user.homePath) | ||
|
||
def test_getUserNonexistentDatabase(self): | ||
""" | ||
The nesting of callDeprecated with assertRaises can result in ugly code. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per https://jml.io/test-docstrings/ this should say what is being tested, not how ugly we consider the aesthetics of the result |
||
|
||
This is an example of nesting. | ||
""" | ||
db = checkers.FilePasswordDB('test_thisbetternoteverexist.db') | ||
self.assertRaises( | ||
error.UnauthorizedLogin, | ||
self.callDeprecated, | ||
Version("Twisted", 1, 2, 0), | ||
db.getUser, 'user', | ||
) | ||
adiroiban marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
Tests which need to use deprecated classes should use the :py:meth:`getDeprecatedModuleAttribute <twisted.trial.unittest.SynchronousTestCase.getDeprecatedModuleAttribute>` helper. | ||
|
||
|
@@ -573,3 +608,41 @@ Tests which need to use deprecated classes should use the :py:meth:`getDeprecate | |
creds = UsernameHashedPassword(b"foo", b"bar") | ||
self.assertEqual(creds.username, b"foo") | ||
self.assertEqual(creds.hashed, b"bar") | ||
|
||
|
||
Deprecation for whole packages or modules can be tested using the `importlib.reload` helper. | ||
|
||
.. code-block:: python | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For new code blocks, we should prefer |
||
|
||
from importlib import reload | ||
|
||
import twisted.old_code | ||
from twisted import old_code | ||
from twisted.trial import unittest | ||
|
||
|
||
class OldCodeDeprecationTests(unittest.TestCase): | ||
""" | ||
Ensures that importing twisted.old_code directly or as a | ||
module of twisted raises a deprecation warning. | ||
""" | ||
|
||
def test_deprecationDirect(self) -> None: | ||
""" | ||
A direct import will raise the deprecation warning. | ||
""" | ||
reload(twisted.old_code) | ||
[warning] = self.flushWarnings([self.test_deprecationDirect]) | ||
self.assertEqual( | ||
"twisted.old_code was deprecated at Twisted NEXT", warnings["message"] | ||
) | ||
|
||
def test_deprecationSubModule(self) -> None: | ||
""" | ||
An import as a sub-module will raise the deprecation warning. | ||
""" | ||
reload(old_code) | ||
[warning] = self.flushWarnings([self.test_deprecationSubModule]) | ||
self.assertEqual( | ||
"twisted.old_code was deprecated at Twisted NEXT", warnings["message"] | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
The development documentation for deprecating API was updated to cover | ||
examples on how to deprecate a whole module or a class and test the deprecation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be translated from passive to active voice and put at the top of the documentation. If we're going to document multiple ways to do something, it should be clear why multiple approaches exist and which one is appropriate in what circumstances.