Skip to content

gh-118331: Fix test_list.ListTest.test_no_memory under trace refs build #130921

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

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

mpage
Copy link
Contributor

@mpage mpage commented Mar 6, 2025

Memory allocation ends up failing in _PyRefchainTrace(), which produces different output. Assert that we don't segfault, which is the thing we want to test and is less brittle than checking output.

Memory allocation ends up failing in _PyRefchainTrace(), which produces
different output. Assert that we don't segfault, which is the thing
we want to test and is less brittle than checking output.
@bedevere-app bedevere-app bot added the tests Tests in the Lib/test dir label Mar 6, 2025
@mpage mpage added the skip news label Mar 6, 2025
@mpage mpage requested review from colesbury, corona10 and encukou March 6, 2025 18:55
@mpage mpage marked this pull request as ready for review March 6, 2025 18:56
@@ -324,8 +325,12 @@ def test_no_memory(self):
_testcapi.set_nomemory(0)
l = [None]
""")
_, _, err = assert_python_failure("-c", code)
self.assertIn("MemoryError", err.decode("utf-8"))
rc, _, _ = assert_python_failure("-c", code)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we mark the test with @unittest.skipIf(support.Py_TRACE_REFS, 'cannot test Py_TRACE_REFS build') so that we don't run it under --with-trace-refs?

That's what we do in test_repl's test_no_memory (a different test with the same name).

Along those lines, I'm a bit confused because the PR refers to #118331, but that's a bug report for test_no_memory in test_repl.py not the test_no_memory in test_list.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we mark the test with @unittest.skipIf(support.Py_TRACE_REFS, 'cannot test Py_TRACE_REFS build') so that we don't run it under --with-trace-refs?

Sure, I have a slight preference for this change since it's testing the thing we care about (not segfaulting) and works in both builds, but that's fine with me.

Along those lines, I'm a bit confused because the PR refers to #118331, but that's a bug report for test_no_memory in test_repl.py not the test_no_memory in test_list.py.

The PR that introduced the failure under tracerefs is linked to that issue so I figured it was fine to link to it as well. I'll create a new issue for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay that makes sense. It doesn't need a new issue

@mpage mpage merged commit 6c6600f into python:main Mar 6, 2025
48 checks passed
seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
…refs build (python#130921)

Fix `test_list.ListTest.test_no_memory` under trace refs build

Memory allocation ends up failing in _PyRefchainTrace(), which produces
different output. Assert that we don't segfault, which is the thing
we want to test and is less brittle than checking output.
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.

2 participants