Revive and enhance test_outparam. (#897)

* test: Unskip `test_c_char` in `test_outparam` as it now passes.

The `test_c_char` in `test_outparam.py` was previously skipped due to
"failing for mysterious reasons". It now passes without modifications.
This suggests that underlying issues might have been resolved by this
package's community or within CPython over the past few years.

* test: Remove redundant `str` conversion from `comstring` in `test_outparam`.

Removed the `text_type = str` assignment and its usage in `comstring` function.
This change eliminates a redundant `str` conversion that remained from Python
2/3 version bridging.

* refactor: Use `logging` module for debug output in `test_outparam`.

Replaced `print` statements with `logger.debug` in `test_outparam.py` to allow
for more granular control and suppression of debug messages.

* test: Replace `ValueError` with `assert` for memory allocation check in `test_outparam`.

Changed the memory allocation check in `test_outparam.py` from raising a
`ValueError` to using an `assert` statement. In test code, avoiding `if`
branches is preferred.

* test: Verify `c_wchar_p` constructor does not allocate memory via COM allocator.

Added an assertion to `test_outparam.py` to verify that `c_wchar_p`'s
constructor does not allocate memory using `CoTaskMemAlloc`. This ensures
the memory is not identified as COM-allocated.
This check is vital to prevent the patched `__ctypes_from_outparam__` from
attempting to free unmanaged memory, which it expects to be COM-allocated.

* test: Remove irrelevant `BSTR` comment from `test_outparam`.

Removed a commented-out `BSTR` instantiation from `test_outparam.py`.
While its presence might have been intended to prevent some type definition
regression, it felt largely unrelated to the core purpose of this specific
test and was therefore removed for clarity.

* test: Refactor `test_c_char` to use `subTest` and verify memory deallocation.

Refactored `test_c_char` in `test_outparam.py` to utilize `self.subTest` for
improved test granularity. The test now explicitly verifies memory allocation
status using `malloc.DidAlloc` before and after calling
`__ctypes_from_outparam__`, ensuring correct memory deallocation behavior.
Old commented-out debug statements were also removed.

* docs: Remove outdated TODO comment from `test_outparam`.

Removed the `TODO` comment in `comtypes/test/test_outparam.py` regarding
"untested changes" and "global effects on other tests". This comment is
no longer relevant due to recent improvements and the unskipping of
`test_c_char`.

* fix: Rename `from_outparm` to `from_outparam` in `test_outparam.py`.

Corrected a typo in the function name `from_outparm` to `from_outparam`
within `test_outparam.py`.

Author: junkmd

comtypes/test/test_outparam.py

@@ -1,3 +1,4 @@
+import logging
 import unittest
 from ctypes import (
     HRESULT,
@@ -22,7 +23,7 @@
 from comtypes import COMMETHOD, GUID, IUnknown
 from comtypes.GUID import _CoTaskMemFree
 
-text_type = str
+logger = logging.getLogger(__name__)
 
 
 class IMalloc(IUnknown):
@@ -55,48 +56,56 @@ class IMalloc(IUnknown):
 assert bool(malloc)
 
 
-def from_outparm(self):
+def from_outparam(self):
     if not self:
         return None
     result = wstring_at(self)
-    if not malloc.DidAlloc(self):
-        raise ValueError("memory was NOT allocated by CoTaskMemAlloc")
+    # `DidAlloc` method returns;
+    # *  1 (allocated)
+    # *  0 (not allocated)
+    # * -1 (cannot determine or NULL)
+    # https://learn.microsoft.com/en-us/windows/win32/api/objidl/nf-objidl-imalloc-didalloc
+    assert malloc.DidAlloc(self), "memory was NOT allocated by CoTaskMemAlloc"
     _CoTaskMemFree(self)
     return result
 
 
 def comstring(text, typ=c_wchar_p):
-    text = text_type(text)
     size = (len(text) + 1) * sizeof(c_wchar)
     mem = _CoTaskMemAlloc(size)
-    print("malloc'd 0x%x, %d bytes" % (mem, size))
+    logger.debug("malloc'd 0x%x, %d bytes" % (mem, size))
     ptr = cast(mem, typ)
     memmove(mem, text, size)
     return ptr
 
 
 class Test(unittest.TestCase):
-    @unittest.skip("This fails for reasons I don't understand yet")
-    # TODO untested changes; this was modified because it had global effects on other tests
-    @patch.object(c_wchar_p, "__ctypes_from_outparam__", from_outparm)
+    @patch.object(c_wchar_p, "__ctypes_from_outparam__", from_outparam)
     def test_c_char(self):
-        # ptr = c_wchar_p("abc")
-        # self.failUnlessEqual(ptr.__ctypes_from_outparam__(),
-        #                         "abc")
-
-        # p = BSTR("foo bar spam")
+        ptr = c_wchar_p("abc")
+        # The normal constructor does not allocate memory using `CoTaskMemAlloc`.
+        # Therefore, calling the patched `ptr.__ctypes_from_outparam__()` would
+        # attempt to free invalid memory, potentially leading to a crash.
+        self.assertEqual(malloc.DidAlloc(ptr), 0)
 
         x = comstring("Hello, World")
         y = comstring("foo bar")
         z = comstring("spam, spam, and spam")
 
-        # (x.__ctypes_from_outparam__(), x.__ctypes_from_outparam__())
-        print((x.__ctypes_from_outparam__(), None))  # x.__ctypes_from_outparam__())
-
-        # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__()
-        # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__()
-        # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__()
-        # print comstring("Hello, World", c_wchar_p).__ctypes_from_outparam__()
+        # The `__ctypes_from_outparam__` method is called to convert an output
+        # parameter into a Python object. In this test, the custom
+        # `from_outparam` function not only converts the `c_wchar_p` to a
+        # Python string but also frees the associated memory. Therefore, it can
+        # only be called once for each allocated memory block.
+        for wchar_ptr, expected in [
+            (x, "Hello, World"),
+            (y, "foo bar"),
+            (z, "spam, spam, and spam"),
+        ]:
+            with self.subTest(wchar_ptr=wchar_ptr, expected=expected):
+                self.assertEqual(malloc.DidAlloc(wchar_ptr), 1)
+                self.assertEqual(wchar_ptr.__ctypes_from_outparam__(), expected)
+                self.assertEqual(malloc.DidAlloc(wchar_ptr), 0)
 
 
 if __name__ == "__main__":