Skip to content

Conversation

@pareshjoshij
Copy link

@pareshjoshij pareshjoshij commented Nov 16, 2025

Closes #141612.

This PR makes the test_trampoline_works_with_forks test in Lib/test/test_perf_profiler.py more robust by adding missing assertions.

The original test only verified the presence of symbols in their respective perf maps (parent symbols in parent map, child symbols in child map). It did not verify the absence of those symbols in the other process's map.

This change adds six self.assertNotIn calls to complete the test logic. It now verifies:

  • Parent-specific symbols (e.g., py::foo) are not in the child's perf map.
  • Child-specific symbols (e.g., py::foo_fork) are not in the parent's perf map.

This closes the gap in the test logic and prevents potential false positives. The test_perf_profiler test suite passes successfully with these changes.

@bedevere-app bedevere-app bot added awaiting review tests Tests in the Lib/test dir labels Nov 16, 2025
@bedevere-app
Copy link

bedevere-app bot commented Nov 16, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@pareshjoshij
Copy link
Author

Hi @picnixz Sir ! , could you please review this when you get a moment?
It’s a small test-only improvement adding missing assertNotIn checks.
If everything looks good, kindly approve/merge.
Thank you! 🙏

pareshjoshij and others added 3 commits November 17, 2025 13:52
Co-authored-by: Mikhail Efimov <efimov.mikhail@gmail.com>
Co-authored-by: Mikhail Efimov <efimov.mikhail@gmail.com>
@efimov-mikhail efimov-mikhail added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Nov 17, 2025
Add assertions for child performance file contents.
Ensure that the child performance file does not contain parent symbols.
@efimov-mikhail
Copy link
Member

@pareshjoshij You can see, what form of result I suspect:
https://github.com/efimov-mikhail/cpython/commits/fix-perf-test-assertions/

@pareshjoshij
Copy link
Author

So sorry @efimov-mikhail sir about that! :(

I tried to apply the fix via the GitHub web UI because I am currently out of town and away from my development setup. Unfortunately, it looks like the web editor mishandled the indentation.

I will fix this properly as soon as I get back to my computer. That said, if you'd prefer not to wait, please feel free to commit the edit directly—I have 'Allow edits from maintainers' enabled.

@efimov-mikhail
Copy link
Member

Don't worry, it's not a problem and there's no hurry.

Copy link
Member

@efimov-mikhail efimov-mikhail left a comment

Choose a reason for hiding this comment

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

LGTM.

Could you please take a look, @picnixz ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests: test_trampoline_works_with_forks assertions are incomplete

3 participants