Revive and enhance test_outparam.
#897
Merged
Conversation
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
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.
…param`. 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.
Replaced `print` statements with `logger.debug` in `test_outparam.py` to allow for more granular control and suppression of debug messages.
…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.
… 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.
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.
…ocation. 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.
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`.
Corrected a typo in the function name `from_outparm` to `from_outparam` within `test_outparam.py`.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #897 +/- ##
==========================================
+ Coverage 85.40% 85.55% +0.14%
==========================================
Files 126 126
Lines 12018 12022 +4
==========================================
+ Hits 10264 10285 +21
+ Misses 1754 1737 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request re-enables and refactors the
test_c_charintest_outparam.py, which was previously skipped "for misterious reasons"(3cadc71, #298, #271).It is unclear why this test has started working again without any direct changes. Our best guess is that a combination of CPython bugfix releases and community-led fixes like #473 has resolved the underlying issue.
Test Enhancement:
The re-enabled
test_c_charnow provides verification for the correct handling pointers and their associated memory management within COM interoperability.The explicit checks for
malloc.DidAllocdirectly address the nuances of COM memory ownership.Refactoring Value:
The test code has been improved for clarity, robustness, and maintainability.
self.subTestprovides granular feedback for each test case, making it easier to identify and debug specific failures.ValueErrorto directassertstatements aligns with idiomatic Python testing practices, simplifying test logic.printstatements tologger.debugallows for better control over debugging output, improving the signal-to-noise ratio during test execution.strconversion and irrelevant comments streamlines the codebase.