Skip to content
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

Add ability to wholesale silence DeprecationWarnings while running the test suite #89015

Closed
ambv opened this issue Aug 6, 2021 · 11 comments
Closed
Assignees
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@ambv
Copy link
Contributor

ambv commented Aug 6, 2021

BPO 44852
Nosy @ambv, @serhiy-storchaka, @miss-islington
PRs
  • bpo-44852: Support ignoring specific DeprecationWarnings wholesale in regrtest #27634
  • [3.10] bpo-44852: Support ignoring specific DeprecationWarnings wholesale in regrtest (GH-27634) #27784
  • [3.9] bpo-44852: Support ignoring specific DeprecationWarnings wholesale in regrtest (GH-27634) #27785
  • bpo-44852: Support filtering over warnings without a set message #27793
  • [3.10] bpo-44852: Support filtering over warnings without a set message (GH-27793) #27809
  • [3.9] bpo-44852: Support filtering over warnings without a set message (GH-27793) #27810
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ambv'
    closed_at = <Date 2021-08-17.10:01:37.964>
    created_at = <Date 2021-08-06.16:34:02.574>
    labels = ['type-feature', 'tests', '3.9', '3.10', '3.11']
    title = 'Add ability to wholesale silence DeprecationWarnings while running the test suite'
    updated_at = <Date 2021-08-18.12:10:43.260>
    user = 'https://github.com/ambv'

    bugs.python.org fields:

    activity = <Date 2021-08-18.12:10:43.260>
    actor = 'lukasz.langa'
    assignee = 'lukasz.langa'
    closed = True
    closed_date = <Date 2021-08-17.10:01:37.964>
    closer = 'lukasz.langa'
    components = ['Tests']
    creation = <Date 2021-08-06.16:34:02.574>
    creator = 'lukasz.langa'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 44852
    keywords = ['patch']
    message_count = 11.0
    messages = ['399102', '399104', '399118', '399150', '399156', '399665', '399680', '399728', '399836', '399840', '399841']
    nosy_count = 3.0
    nosy_names = ['lukasz.langa', 'serhiy.storchaka', 'miss-islington']
    pr_nums = ['27634', '27784', '27785', '27793', '27809', '27810']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue44852'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @ambv
    Copy link
    Contributor Author

    ambv commented Aug 6, 2021

    Sometimes we have the following problem:

    • there are way too many deprecations raised from a given library to silence them one by one;
    • the deprecations are different between maintenance branches and we don't want to make tests between the branches hopelessly conflicting.

    In particular, there is such a case right now with asyncio in 3.9 vs. later branches. 3.8 deprecated the loop= argument in a bunch of functions but due to poor warning placement, most of them were silent. This is being fixed in BPO-44815 which would go out to users in 3.9.7 but that created 220 new warnings when running test_asyncion in regression tests. Fixing them one by one would be both tedious, and would make the 3.9 branch forever conflicting with newer branches in many asyncio test files. In 3.11 there's a new round of deprecations raised in test_asyncio, making the branches different. Moreover, those warnings are typically silenced by assertWarns context managers which should only be used when actually testing the warnings, not to silence irrelevant warnings.

    So, what the PR does is it introduces:

    • support.ignore_deprecations_from("path.to.module", like=".*msg regex.*"), and
    • support.clear_ignored_deprecations()

    The former adds a new filter to warnings, the message regex is mandatory. The latter removes only the filters that were added by the former, leaving all other filters alone.

    Example usage is in test_support, and later, should this be merged, will be in asyncio tests on the 3.9 branch.

    @ambv ambv added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes labels Aug 6, 2021
    @ambv ambv self-assigned this Aug 6, 2021
    @ambv ambv added tests Tests in the Lib/test dir type-feature A feature request or enhancement 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes labels Aug 6, 2021
    @ambv ambv self-assigned this Aug 6, 2021
    @ambv ambv added tests Tests in the Lib/test dir type-feature A feature request or enhancement labels Aug 6, 2021
    @ambv
    Copy link
    Contributor Author

    ambv commented Aug 6, 2021

    For example, all that's needed to silence the 220 new warnings in all asyncio tests, is adding this in Lib/test/test_asyncio/init.py:

    def setUpModule():
        support.ignore_deprecations_from("asyncio.base_events", like=r".*loop argument.*")
        support.ignore_deprecations_from("asyncio.unix_events", like=r".*loop argument.*")
        support.ignore_deprecations_from("asyncio.futures", like=r".*loop argument.*")
        support.ignore_deprecations_from("asyncio.runners", like=r".*loop argument.*")
        support.ignore_deprecations_from("asyncio.subprocess", like=r".*loop argument.*")
        support.ignore_deprecations_from("asyncio.tasks", like=r".*loop argument.*")
        support.ignore_deprecations_from("test.test_asyncio.test_queues", like=r".*loop argument.*")
        support.ignore_deprecations_from("test.test_asyncio.test_tasks", like=r".*loop argument.*")
    
    def tearDownModule():
        support.clear_ignored_deprecations()
    

    Since the __init__.py file in question isn't a module that runs tests itself but only a package gathering many sub-modules, we need some boilerplate like:

    def load_tests(loader, _, pattern):
        pkg_dir = os.path.dirname(__file__)
        suite = AsyncioTestSuite()
        return support.load_package_tests(pkg_dir, loader, suite, pattern)
    
    
    class AsyncioTestSuite(unittest.TestSuite):
        """A custom test suite that also runs setup/teardown for the whole package.
    
        Normally unittest only runs setUpModule() and tearDownModule() within each
        test module part of the test suite. Copying those functions to each file
        would be tedious, let's run this once and for all.
        """
        def run(self, result, debug=False):
            try:
                setUpModule()
                super().run(result, debug=debug)
            finally:
                tearDownModule()
    

    With that, all of asyncio tests silence unnecessary deprecation warnings. Additionally, testing for warnings with warnings_helper.check_warnings() or assertWarns still works just fine as those facilities temporarily disable filtering.

    @serhiy-storchaka
    Copy link
    Member

    There are problems with clearing all ignored deprecation filters.

    We can want to ignore warnings in narrower or wider scope: per-method, per-class, per-module. We should use the warnings.catch_warnings() context manager for restoring filters in the correct scope. For example, use setUp/tearDown, setUpClass/tearDownClass, setUpModule/tearDownModule and save the context manager as an instance/class attribute or module global.

       def setUp(self):
           self.w = warnings.catch_warnings()
           self.w.__enter__()
           warnings.filterwarnings(...)
    
        def tearDown():
            self.w.__exit__(None, None, None)

    It is hard to use any helper here because the code should be added in multiple places.

    Or use setUp/addCleanup, setUpClass/addClassCleanup, setUpModule/addModuleCleanup if you want to keep all code in one place:

       def setUp(self):
           w = warnings.catch_warnings()
           w.__enter__()
           self.addCleanup(w.__exit__, None, None, None)
           warnings.filterwarnings(...)

    The helper can call catch_warnings(), __enter__(), set filters and return a caller which calls __exit__(None, None, None):

       def setUp(self):
           self.addCleanup(some_helper(...))

    For class and method scopes it would be convenient to use class and method decorators correspondingly.

        @ignore_some_warnings(...)
        def test_foo(self):
            ...
    
    @ignore_some_warnings(...)
    class BarTests(unittest.TestCase):
        ...

    @ambv
    Copy link
    Contributor Author

    ambv commented Aug 6, 2021

    Serhiy, if you take a look at the code, clear_ignored_deprecations() only clears the filters that were added by ignore_deprecations_from(). It doesn't touch other filters. Sure, we can abuse the context manager instead but this looks pretty busy:

       def setUp(self):
           self.w = warnings.catch_warnings()
           self.w.__enter__()
           warnings.filterwarnings(...)
    
        def tearDown():
            self.w.__exit__(None, None, None)

    Making the API symmetrical is a good idea though, I'll look into that tomorrow.

    @serhiy-storchaka
    Copy link
    Member

    clear_ignored_deprecations() does not know whether filters were added in setUp(), setUpClass(), setUpModule() or just in a particular method. We should use teadDown*() or add*Cleanup() to clean up at appropriate level.

    @ambv
    Copy link
    Contributor Author

    ambv commented Aug 16, 2021

    New changeset a0a6d39 by Łukasz Langa in branch 'main':
    bpo-44852: Support ignoring specific DeprecationWarnings wholesale in regrtest (GH-27634)
    a0a6d39

    @ambv
    Copy link
    Contributor Author

    ambv commented Aug 16, 2021

    New changeset c7c4cbc by Łukasz Langa in branch '3.9':
    [3.9] bpo-44852: Support ignoring specific DeprecationWarnings wholesale in regrtest (GH-27634) (GH-27785)
    c7c4cbc

    @ambv
    Copy link
    Contributor Author

    ambv commented Aug 17, 2021

    New changeset bc98f98 by Łukasz Langa in branch '3.10':
    [3.10] bpo-44852: Support ignoring specific DeprecationWarnings wholesale in regrtest (GH-27634) (GH-27784)
    bc98f98

    @ambv ambv closed this as completed Aug 17, 2021
    @ambv ambv closed this as completed Aug 17, 2021
    @ambv
    Copy link
    Contributor Author

    ambv commented Aug 18, 2021

    New changeset 8cf07d3 by Łukasz Langa in branch 'main':
    bpo-44852: Support filtering over warnings without a set message (GH-27793)
    8cf07d3

    @miss-islington
    Copy link
    Contributor

    New changeset d1c0e44 by Miss Islington (bot) in branch '3.10':
    bpo-44852: Support filtering over warnings without a set message (GH-27793)
    d1c0e44

    @ambv
    Copy link
    Contributor Author

    ambv commented Aug 18, 2021

    New changeset ebe7e6d by Miss Islington (bot) in branch '3.9':
    bpo-44852: Support filtering over warnings without a set message (GH-27793) (GH-27810)
    ebe7e6d

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes tests Tests in the Lib/test dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants