Repository navigation
Add record SAFEARRAY tests and a test-double COM server. - #980
Merged
junkmd merged 4 commits intoNov 2, 2026
Merged
Conversation
…enthought#63. Some of the dispatch-path tests that use `assertRaises` pin down the current behavior, which is a known limitation rather than the intended contract. Unlike the cases where the server legitimately rejects the input (e.g. empty arrays returning `E_INVALIDARG`), those should become normal-path tests once enthought#63 is fixed. The `CHARACTERIZATION(enthought#63)` message is reported only when the call no longer raises, which is exactly when someone has fixed the issue, so the failure itself tells them what to do next.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #980 +/- ##
==========================================
+ Coverage 90.50% 90.76% +0.26%
==========================================
Files 145 147 +2
Lines 14831 15271 +440
==========================================
+ Hits 13423 13861 +438
- Misses 1408 1410 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Arrays of records (
VT_ARRAY | VT_RECORD) are only partially supported bycomtypestoday.Through a typed (vtable) interface, record arrays go in and come out correctly.
Through
IDispatch::Invovke, they can be sent to a server as typedSAFEARRAYs, but a record array coming back fails withKeyError: 36(#63; #104 proposes unpacking them).What is missing is not only the fix, but a way to know whether any change in this area is correct.
A fix for #63 touches the code that unpacks record arrays, and that code is shared with the vtable path, which works today.
Without a baseline, a change can look right on the path it targets while silently regressing the other one.
It is also hard to test against existing COM components: nothing predictable accepts and returns record arrays with known contents.
This PR adds that baseline before touching any library code, for three reasons:
Some
assertRaisescases are intended (e.g., the server rejects empty arrays withE_INVALIDARG). Others document a known limitation (VARIANT._get_valuecannot unpack arrays of arbitrary type #63) and are markedCHARACTERIZATION(#63).The
CHARACTERIZATION(#63)marker is set in themsgargument ofassertRaises, so it appears only when the call stops raising exceptions (i.e., whenVARIANT._get_valuecannot unpack arrays of arbitrary type #63 is fixed). When someone fixesVARIANT._get_valuecannot unpack arrays of arbitrary type #63, this failure will remind them to update the test to a normal test case.The server method returns the input array in reverse, so the expected value is simply
reversed(records). Using arrays with multiple elements also reveals issues that single-element arrays would conceal.Two record kinds are tested on purpose. Integer-only records isolate marshaling. BSTR records add ownership of heap-allocated strings, which is where unpacking changes are most likely to go wrong.
Summary
IDualRecordSafearrayTest(vtable) andIDispRecordSafearrayTest(dispatch).What this does not do:
Related: #63, #104. This does not close either.