From a0ccef78436d7abe4339458a3cb16747de6da2f6 Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Fri, 8 Sep 2023 22:52:41 +0100 Subject: [PATCH 1/3] Initial update for testing deprecated modules. --- docs/development/compatibility-policy.rst | 84 +++++++++++++++++++++-- src/twisted/newsfragments/8478.doc | 2 + 2 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 src/twisted/newsfragments/8478.doc diff --git a/docs/development/compatibility-policy.rst b/docs/development/compatibility-policy.rst index 0f5d6de9402..a4a17e88700 100644 --- a/docs/development/compatibility-policy.rst +++ b/docs/development/compatibility-policy.rst @@ -319,6 +319,8 @@ Classes Classes are deprecated by raising a warning when they are access from within their module, using the :py:func:`deprecatedModuleAttribute ` helper. +This can trigger the warnings at important time. + .. code-block:: python class SSLContextFactory: @@ -445,12 +447,33 @@ Modules cannot have properties, so module attributes should be deprecated using Modules ^^^^^^^ -To deprecate an entire module, :py:func:`deprecatedModuleAttribute ` can be used on the parent package's ``__init__.py``. +To deprecate an entire module there are two 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 `. + +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 is now deprecated. + """ + import warnings -There are two other options: + from incremental import Version, getVersionString -* Put a warnings.warn() call into the top-level code of the module. -* Deprecate all of the attributes of the module. + 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 +574,19 @@ 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. + + This is an example of nesting. + """ + self.db = checkers.FilePasswordDB('test_thisbetternoteverexist.db') + self.assertRaises( + error.UnauthorizedLogin, + self.callDeprecated, + Version("Twisted", 1, 2, 0), + self.db.getUser, 'user', + ) Tests which need to use deprecated classes should use the :py:meth:`getDeprecatedModuleAttribute ` helper. @@ -573,3 +609,43 @@ 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 + + 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) + warnings = self.flushWarnings([self.test_deprecationDirect]) + self.assertEqual(1, len(warnings)) + self.assertEqual( + "twisted.old_code was deprecated at Twisted NEXT", warnings[0]["message"] + ) + + def test_deprecationSubModule(self) -> None: + """ + An import as a sub-module will raise the deprecation warning. + """ + reload(old_code) + warnings = self.flushWarnings([self.test_deprecationSubModule]) + self.assertEqual(1, len(warnings)) + self.assertEqual( + "twisted.old_code was deprecated at Twisted NEXT", warnings[0]["message"] + ) diff --git a/src/twisted/newsfragments/8478.doc b/src/twisted/newsfragments/8478.doc new file mode 100644 index 00000000000..aa1d87a9700 --- /dev/null +++ b/src/twisted/newsfragments/8478.doc @@ -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. From 52ceab60c27c627879bcececcec0ec20fd67255f Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Sun, 24 Sep 2023 10:20:19 +0100 Subject: [PATCH 2/3] Apply suggestions from code review by twm Co-authored-by: Tom Most --- docs/development/compatibility-policy.rst | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/docs/development/compatibility-policy.rst b/docs/development/compatibility-policy.rst index a4a17e88700..92d58e238ed 100644 --- a/docs/development/compatibility-policy.rst +++ b/docs/development/compatibility-policy.rst @@ -319,8 +319,6 @@ Classes Classes are deprecated by raising a warning when they are access from within their module, using the :py:func:`deprecatedModuleAttribute ` helper. -This can trigger the warnings at important time. - .. code-block:: python class SSLContextFactory: @@ -580,13 +578,14 @@ Making calls to the deprecated code without raising these warnings can be done u This is an example of nesting. """ - self.db = checkers.FilePasswordDB('test_thisbetternoteverexist.db') + db = checkers.FilePasswordDB('test_thisbetternoteverexist.db') self.assertRaises( error.UnauthorizedLogin, self.callDeprecated, Version("Twisted", 1, 2, 0), - self.db.getUser, 'user', + db.getUser, 'user', ) + ) Tests which need to use deprecated classes should use the :py:meth:`getDeprecatedModuleAttribute ` helper. @@ -633,10 +632,9 @@ Deprecation for whole packages or modules can be tested using the `importlib.rel A direct import will raise the deprecation warning. """ reload(twisted.old_code) - warnings = self.flushWarnings([self.test_deprecationDirect]) - self.assertEqual(1, len(warnings)) + [warning] = self.flushWarnings([self.test_deprecationDirect]) self.assertEqual( - "twisted.old_code was deprecated at Twisted NEXT", warnings[0]["message"] + "twisted.old_code was deprecated at Twisted NEXT", warnings["message"] ) def test_deprecationSubModule(self) -> None: @@ -644,8 +642,7 @@ Deprecation for whole packages or modules can be tested using the `importlib.rel An import as a sub-module will raise the deprecation warning. """ reload(old_code) - warnings = self.flushWarnings([self.test_deprecationSubModule]) - self.assertEqual(1, len(warnings)) + [warning] = self.flushWarnings([self.test_deprecationSubModule]) self.assertEqual( - "twisted.old_code was deprecated at Twisted NEXT", warnings[0]["message"] + "twisted.old_code was deprecated at Twisted NEXT", warnings["message"] ) From 6a44cd7c8bf277bd7c3849664ad9443809a0e7d8 Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Sun, 24 Sep 2023 10:31:26 +0100 Subject: [PATCH 3/3] Include deprecated version in module docstring. Co-authored-by: Tom Most --- docs/development/compatibility-policy.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/compatibility-policy.rst b/docs/development/compatibility-policy.rst index 92d58e238ed..27852944839 100644 --- a/docs/development/compatibility-policy.rst +++ b/docs/development/compatibility-policy.rst @@ -462,7 +462,7 @@ This is why it's recommended to just use `warnings.warn()` at the top level of t """ The normal docstring of the top-level package. - This package is now deprecated. + This package was deprecated in Twisted NEXT """ import warnings