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

Allow the definition of the _midlSAFEARRAY(HRESULT) type. #670

Merged
merged 4 commits into from
Nov 27, 2024

Conversation

davidschranz
Copy link
Contributor

Pull request for changes described in #668

Copy link
Collaborator

@junkmd junkmd left a comment

Choose a reason for hiding this comment

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

Thank you for contributing.

We need to write test code as well.

The test will include assertions ensuring that the definition of _midlSAFEARRAY(HRESULT) does not cause an error, while calling _midlSAFEARRAY(HRESULT).create raises a TypeError.

There are multiple modules for testing safearray, so I am currently looking for the most appropriate place to add the test.

# safearray pointer instance does not work.
# See also: https://github.com/enthought/comtypes/issues/668
"Cannot create SAFEARRAY type VT_HRESULT."
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Formatting issue.

-                    )
+                )

Copy link
Collaborator

@junkmd junkmd left a comment

Choose a reason for hiding this comment

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

I believe that test/test_midl_safearray_create.py is the most suitable location to test this behavior.

You could add a test like the following:

# coding: utf-8
from ctypes import c_int, pointer, POINTER
import unittest

 # coding: utf-8

-from ctypes import c_int, pointer, POINTER
+from ctypes import c_int, pointer, HRESULT, POINTER
 import unittest

self.assertEqual(unpacked.answer, 42)
self.assertEqual(unpacked.needs_clarification, True)
def test_ctype(self):
extra = None
cdata = c_int(1)

                 self.assertEqual(unpacked.answer, 42)
                 self.assertEqual(unpacked.needs_clarification, True)

+    def test_HRESULT(self):
+        hr = HRESULT(1)
+        sa_type = comtypes.safearray._midlSAFEARRAY(HRESULT)
+        with self.assertRaises(TypeError):
+            sa_type.create([hr], extra=None)
+        with self.assertRaises(TypeError):
+            sa_type.create([hr])
+
     def test_ctype(self):
         extra = None
         cdata = c_int(1)

junkmd added a commit to junkmd/pywinauto that referenced this pull request Nov 27, 2024
@junkmd junkmd changed the title KeyError: <class 'ctypes.HRESULT'>, _ctype_to_vartype, HRESULT: VT_HRESULT Allow the definition of the _midlSAFEARRAY(HRESULT) type. Nov 27, 2024
junkmd added a commit to junkmd/pywinauto that referenced this pull request Nov 27, 2024
@junkmd junkmd added this to the 1.4.9 milestone Nov 27, 2024
@junkmd junkmd added the enhancement New feature or request label Nov 27, 2024
Copy link
Collaborator

@junkmd junkmd left a comment

Choose a reason for hiding this comment

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

Thanks.

@junkmd junkmd merged commit 7b31899 into enthought:main Nov 27, 2024
49 checks passed
@davidschranz
Copy link
Contributor Author

Thank you very much for your help!

@junkmd
Copy link
Collaborator

junkmd commented Nov 28, 2024

You're welcome.
This time, the changes to the production code were minimal, and the tests were also kept to a minimum.
Above all, your excellent reproducer and your responsiveness when I asked for verification made this PR much easier to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KeyError: <class 'ctypes.HRESULT'>, _ctype_to_vartype, HRESULT: VT_HRESULT
2 participants