Skip to content

Conversation

sergey-miryanov
Copy link
Contributor

@sergey-miryanov sergey-miryanov commented Jul 4, 2025

@sergey-miryanov
Copy link
Contributor Author

I believe this PR should skip news

@nascheme
Copy link
Member

nascheme commented Jul 8, 2025

This looks okay to me. Some suggested improvements. The new test might fit better into test_weakref.py but that's minor.

I don't think it's required to run these in a separate Python process. That makes the test run slower and it's not supposed to crash. Generally a separate process is used for the tests that exercise the interpreter shutdown logic and we are not testing that. So you could just do:

    def test_clearing_weakrefs_in_gc(self):
        # This test checks that:
        # 1. weakrefs for types with callbacks are cleared before the
        # finalizer is called
        # 2. weakrefs for types without callbacks are cleared after the
        # finalizer is called
        # 3. other weakrefs cleared before the finalizer is called
        errors = []
        class Class:
            def __init__(self):
                self._self = self
                self._1 = weakref.ref(Class, lambda x: None)
                self._2 = weakref.ref(Class)
                self._3 = weakref.ref(self)

            def __del__(self):
                if self._1() is not None:
                    errors.append("Type weakref with callback is not None as expected")
                if self._2() is not Class:
                    errors.append("Type weakref is not Class as expected")
                if self._3() is not None:
                    errors.append("Instance weakref is not None as expected")
        Class()
        gc.collect()
        self.assertEqual(errors, [])

I find the variable naming a bit esoteric. I would have just called them wr1, wr2 and wr3.

@sergey-miryanov
Copy link
Contributor Author

@nascheme Thanks for review! Fixed.

@sergey-miryanov
Copy link
Contributor Author

@kumaraditya303 Thanks for review!

Types are not treated specially, it only matters if the weakref has a callback or not.
@nascheme nascheme merged commit 25518f5 into python:main Aug 8, 2025
40 checks passed
@sergey-miryanov
Copy link
Contributor Author

Thanks everyone!

@sergey-miryanov sergey-miryanov deleted the gh-135552-add-tests-gc-weakref-tp_subclasses branch August 8, 2025 04:42
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
These are tests to ensure behaviour introduced by pythonGH-136189 is working as expected.

Co-authored-by: Mikhail Borisov <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Neil Schemenauer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants