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

gh-93649: Split tracemalloc tests from _testcapimodule.c #99551

Merged
merged 5 commits into from
Dec 17, 2022

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Nov 16, 2022

Also split memory tests from Lib/test/test_capi/test_misc.py

@erlend-aasland erlend-aasland changed the title split testcapi/tracemalloc gh-93649: Split tracemalloc tests from _testcapimodule.c Nov 16, 2022
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

I will have so many merge conflicts in my C coverage PRs, ahaha 🙈

Thanks for working on this!

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@ezio-melotti ezio-melotti left a comment

Choose a reason for hiding this comment

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

The splits don't seem to preserve history.
For test_mem.py it should be possible to preserve it since it's a new file. For mem.c, I think git is supposed to figure it out on its own since the code have been moved but no files have been created. Maybe moving the code in a commit on its own will work?

@erlend-aasland
Copy link
Contributor Author

The splits don't seem to preserve history. For test_mem.py it should be possible to preserve it since it's a new file. For mem.c, I think git is supposed to figure it out on its own since the code have been moved but no files have been created. Maybe moving the code in a commit on its own will work?

Tests with various git log commands:

$ git log --oneline --follow -S hook_malloc Modules | wc -l
4

OTOH, git log -L seems to have problems, but that command does not work with --follow:

$ git log --oneline -L:hook_malloc:Modules/_testcapi/mem.c | wc -l
1

Maybe moving the code in a commit on its own will work?

Possibly, but I won't bet money on it since the commits are squashed upon merge anyway; previous move operations in Lib/test have worked swell (and there's been a lot of that; Serhiy was busy with test organization fairly recently). IMO reorganizing the code like this has far more benefits than disadvantages, since the resulting smaller files are easier to read, understand, and maintain, which again makes it easier to fix bugs.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

Lib/test/test_capi/test_mem.py Outdated Show resolved Hide resolved
Copy link
Contributor

@kumaraditya303 kumaraditya303 left a comment

Choose a reason for hiding this comment

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

LGTM

@erlend-aasland erlend-aasland merged commit 2b38a9a into python:main Dec 17, 2022
@erlend-aasland erlend-aasland deleted the split-testcapi/tracemalloc branch December 17, 2022 18:03
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Dec 18, 2022
* origin/main: (1306 commits)
  Correct CVE-2020-10735 documentation (python#100306)
  pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273)
  pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551)
  Docs: Use `PY_VERSION_HEX` for version comparison (python#100179)
  pythongh-97909: Fix markup for `PyMethodDef` members (python#100089)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205)
  pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469)
  pythongh-99540: Constant hash for _PyNone_Type to aid reproducibility (pythonGH-99541)
  pythongh-100039: enhance __signature__ to work with str and callables (pythonGH-100168)
  pythongh-99830: asyncio: Document returns of remove_{reader,writer} (python#100302)
  "Compound statement" docs: Fix with-statement step indexing (python#100286)
  pythonGH-90043: Handle NaNs in COMPARE_OP_FLOAT_JUMP (pythonGH-100278)
  Improve stats presentation for calls. (pythonGH-100274)
  Better stats for `LOAD_ATTR` and `STORE_ATTR` (pythonGH-100295)
  pythongh-81057: Move the Cached Parser Dummy Name to _PyRuntimeState (python#100277)
  Document that zipfile's pwd parameter is a `bytes` object (python#100209)
  pythongh-99767: mark `PyTypeObject.tp_watched` as internal use only in table (python#100271)
  Fix typo in introduction.rst (python#100266)
  ...
carljm added a commit to carljm/cpython that referenced this pull request Dec 19, 2022
* main:
  pythongh-89727: Fix os.walk RecursionError on deep trees (python#99803)
  Docs: Don't upload CI artifacts (python#100330)
  pythongh-94912: Added marker for non-standard coroutine function detection (python#99247)
  Correct CVE-2020-10735 documentation (python#100306)
  pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273)
  pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551)
  Docs: Use `PY_VERSION_HEX` for version comparison (python#100179)
  pythongh-97909: Fix markup for `PyMethodDef` members (python#100089)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205)
  pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants