#8478 Document testing deprecated modules. #11971
Conversation
|
needs-review |
Co-authored-by: Tom Most <twm@freecog.net>
Co-authored-by: Tom Most <twm@freecog.net>
|
Many thanks Tom for your review. Excellent suggestions. This is ready for another review needs-review |
There was a problem hiding this comment.
I really want to make sure that if we add docs explaining a new option, we explain what the new option is, and clearly lead with why it's preferred. IF we need to still document the old option, then it should be described second, along with a clear explanation of why it's documented.
| 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. |
There was a problem hiding this comment.
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.
| @@ -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.
Per https://jml.io/test-docstrings/ this should say what is being tested, not how ugly we consider the aesthetics of the result
| """ | ||
| The normal docstring of the top-level package. | ||
|
|
||
| This package was deprecated in Twisted NEXT | ||
| """ | ||
| import warnings |
There was a problem hiding this comment.
This module should have an introduction that explains what the code sample is meant to illustrate before launching into it.
|
|
||
| Deprecation for whole packages or modules can be tested using the `importlib.reload` helper. | ||
|
|
||
| .. code-block:: python |
There was a problem hiding this comment.
For new code blocks, we should prefer literalinclude directives so that the Python code lives in Python files which can be loaded and tested.
|
Thanks @glyph for the review. My goal was to get something better than the current documentation ... we can make it perfect in follow up PRs :) I don't think I will have time to look into this soon... maybe next time I will need to deprecated tests for a PR. If anyone wants to continue working on this. No problem. The branch was pushed to the main repo so anyone from the team can write to it. Cheers |
Scope and purpose
Fixes #8478
This document how to test deprecated modules.
it also contains a few changes from a very old branch 7 years old - https://github.com/twisted/twisted/tree/8478-deprecated-class-test
docs at https://twisted--11971.org.readthedocs.build/en/11971/development/compatibility-policy.html#testing-deprecation-code