FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-127183: Add `_ctypes.CopyComPointer` tests by junkmd · Pull Request #127184 · python/cpython · GitHub

/ cpython Public

gh-127183: Add _ctypes.CopyComPointer tests - #127184

Merged
encukou merged 5 commits into
python:mainfrom
junkmd:add_copy_com_pointer_tests
Nov 25, 2024
ConversationCommits (5)ChecksFiles changed
Merged

gh-127183: Add _ctypes.CopyComPointer tests#127184
encukou merged 5 commits into
python:mainfrom
junkmd:add_copy_com_pointer_tests

Conversation

junkmd commented Nov 23, 2024 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

I would like to backport this to 3.12 and 3.13 as well.
This is internal-only, so I don’t think it needs a NEWS entry.

bedevere-app Bot added tests Tests in the Lib/test dir awaiting review labels Nov 23, 2024
bedevere-app Bot mentioned this pull request Nov 23, 2024
Closed
junkmd force-pushed the add_copy_com_pointer_tests branch from f48bcf1 to 638487b Compare November 24, 2024 01:15
encukou reviewed Nov 25, 2024
View reviewed changes

encukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you.
I see CopyComPointer is undocumented. Would you be willing to add some documentation for it as well?

Comment thread Lib/test/test_ctypes/test_win32_com_foreign_func.py Outdated
dst_orig = create_shelllink_persist(self.IPersist)
dst = self.IPersist()
CopyComPointer(dst_orig, byref(dst))
dst_orig.Release() # The refcount of `dst_orig` is 1 here.

encukou Nov 25, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Could the refcount be checked using Release's return value?

junkmd Nov 25, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Indeed, since this assertion is not costly, it is better to simply call assertEqual than to leave a comment about it.
I have changed these lines.

junkmd commented Nov 25, 2024

Copy link
Copy Markdown
Contributor Author

Would you be willing to add some documentation for it as well?

That's my intention, but as discussed in gh-126686, I believe publicizing it is also necessary for proper documentation.

Before moving forward with documentation or publicizing, I want to backport this test to 3.13 and 3.12, where CopyComPointer, albeit private, already exists.
Therefore, I aim to keep this PR in a state where it can be merged as-is when miss-islington backports it.

encukou added skip news needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Nov 25, 2024
encukou merged commit c7f1e3e into python:main Nov 25, 2024

miss-islington-app Bot commented Nov 25, 2024

Copy link
Copy Markdown

Thanks @junkmd for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 25, 2024
pythongh-127183: Add _ctypes.CopyComPointer tests (pythonGH-127184)
8ff7119 
* Make `create_shelllink_persist` top level function.

* Add `CopyComPointerTests`.

* Add more tests.

* Update tests.

* Add assertions for `Release`'s return value.
(cherry picked from commit c7f1e3e)

Co-authored-by: Jun Komoda <45822440+junkmd@users.noreply.github.com>

bedevere-app Bot commented Nov 25, 2024

Copy link
Copy Markdown

GH-127251 is a backport of this pull request to the 3.13 branch.

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Nov 25, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 25, 2024
pythongh-127183: Add _ctypes.CopyComPointer tests (pythonGH-127184)
c334007 
* Make `create_shelllink_persist` top level function.

* Add `CopyComPointerTests`.

* Add more tests.

* Update tests.

* Add assertions for `Release`'s return value.
(cherry picked from commit c7f1e3e)

Co-authored-by: Jun Komoda <45822440+junkmd@users.noreply.github.com>

bedevere-app Bot commented Nov 25, 2024

Copy link
Copy Markdown

GH-127252 is a backport of this pull request to the 3.12 branch.

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Nov 25, 2024

encukou commented Nov 25, 2024

Copy link
Copy Markdown
Member

Makes sense. Thank you for the test!

junkmd deleted the add_copy_com_pointer_tests branch November 25, 2024 14:10
This was referenced Nov 25, 2024
Closed
Merged
encukou pushed a commit that referenced this pull request Nov 26, 2024
[3.13] gh-127183: Add _ctypes.CopyComPointer tests (GH-127184) (GH-…
0f77357 
…127251)

gh-127183: Add `_ctypes.CopyComPointer` tests (GH-127184)

* Make `create_shelllink_persist` top level function.

* Add `CopyComPointerTests`.

* Add more tests.

* Update tests.

* Add assertions for `Release`'s return value.
(cherry picked from commit c7f1e3e)

Co-authored-by: Jun Komoda <45822440+junkmd@users.noreply.github.com>
encukou pushed a commit that referenced this pull request Nov 26, 2024
[3.12] gh-127183: Add _ctypes.CopyComPointer tests (GH-127184) (GH-…
08531ac 
…127252)

gh-127183: Add `_ctypes.CopyComPointer` tests (GH-127184)

* Make `create_shelllink_persist` top level function.

* Add `CopyComPointerTests`.

* Add more tests.

* Update tests.

* Add assertions for `Release`'s return value.
(cherry picked from commit c7f1e3e)

Co-authored-by: Jun Komoda <45822440+junkmd@users.noreply.github.com>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
pythongh-127183: Add _ctypes.CopyComPointer tests (pythonGH-127184)
0d22de4 
* Make `create_shelllink_persist` top level function.

* Add `CopyComPointerTests`.

* Add more tests.

* Update tests.

* Add assertions for `Release`'s return value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

encukou encukou left review comments

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL