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

array of BSTR is not supported #347

Open
matthiasfinger opened this issue Sep 9, 2022 · 11 comments
Open

array of BSTR is not supported #347

matthiasfinger opened this issue Sep 9, 2022 · 11 comments

Comments

@matthiasfinger
Copy link

Hi, I really appreciate the great work to develop comtypes. Many thanks! I miss one feature which is the transfer of BSTR arrays. I suggest to change the following code lines in automation.py:

elif isinstance(value, (list, tuple)):
            isstr = [isinstance(x,str) for x in value]
            if not False in isstr:
                bstrlist = []
                for item in value:
                    bstrlist.append(cast(SysAllocStringLen(item, len(item)),BSTR))
                obj = midlSAFEARRAY(BSTR).create(bstrlist)
            else:
                obj = _midlSAFEARRAY(VARIANT).create(value)
            memmove(byref(self._), byref(obj), sizeof(obj))
            self.vt = VT_ARRAY | obj._vartype_
@junkmd
Copy link
Collaborator

junkmd commented Sep 10, 2022

I think this may be related to #80.

Your proposal should be more based on the new code than #80.

What is the COM library stuff that behaves to return an VARIANT that has a VT_ARRAY | VT_BSTR typecode?

I am concerned about how to test to guarantee that a VARIANT with typecode VT_ARRAY | VT_BSTR will be returned.

@matthiasfinger
Copy link
Author

In my example I send a BSTR array to a library. Unfortunately it is not freely available. Maybe someone else has an example? One solution would be a Python COM server as test. For me the code works fine.

@junkmd
Copy link
Collaborator

junkmd commented Sep 10, 2022

@matthiasfinger

It sounds good to have a COM server library for testing.

It seems that the existing COM server testings were also ran that way.

However, these tests, which require running as an administrator or registering in the registry, have been skipped.

At least, I think that it is necessary that the tests run in AppVeyor CI to ensure behavior.

I would like to hear from anyone familiar with AppVeyor and its admin privileges and the registry.

Or do you have any good way to testing?

@vasily-v-ryabov
Copy link
Collaborator

AppVeyor build agent should run as Administrator with disabled UAC. Just need to register COM servers in appveyor.yml before running the tests. If something is failed, I can take a look.

@matthiasfinger
Copy link
Author

Maybe it would be necessary to implement a way to define the output data type in the future. Just declaring it over the used python type could be insufficient in some cases. For instance by creating and passing a custom safearray. At the moment this possibility is quite hidden and would be needed more at the frontend.

@junkmd
Copy link
Collaborator

junkmd commented Sep 18, 2022

@vasily-v-ryabov

I know very little about the registry and AppVeyor, so I apologize if I am totally off base.

I think it might be able to register any typelib in the registry this way, is this correct?

comtypes/appveyor.yml

Lines 27 to 31 in 7f7c283

test_script:
- C:\%py%\python.exe setup.py install
- C:\%py%\Scripts\pip.exe uninstall comtypes -y
- C:\%py%\python.exe test_pip_install.py
- C:\%py%\python.exe -m unittest discover -v -s ./comtypes/test -t comtypes\test

test_script:
   - cd comtypes\test
   - regsvr32 TestComServer.tlb
   - cd ..\..
   - C:\%py%\python.exe setup.py install
   - C:\%py%\Scripts\pip.exe uninstall comtypes -y
   - C:\%py%\python.exe test_pip_install.py
   - C:\%py%\python.exe -m unittest discover -v -s ./comtypes/test -t comtypes\test

I cannot be assure that the may not be compatible with the version of Windows that you're running error dialog will not appear.

@vasily-v-ryabov
Copy link
Collaborator

regsvr32 supports only .dll files. You need regasm tool: https://learn.microsoft.com/en-us/dotnet/framework/tools/regasm-exe-assembly-registration-tool?redirectedfrom=MSDN You should be careful with 32-bit or 64-bit version of the tool and Python.

Also I'd suggest to use setUpClass and tearDownClass methods to register and unregister type libraries only for relevant test suites. For example, subprocess.check_output("some command", shell=True) can be useful.

@junkmd
Copy link
Collaborator

junkmd commented Nov 20, 2022

Is it corrrect?

# comtypes\test\test_comserver.py
REG_PATH = os.path.join(os.path.dirname(__file__), "TestComServer.tlb")

class TestInproc(unittest.TestCase):
    @classmethod
    def setUpClass(cls):
        subprocess.check_output("regsam \"%s\"" % REG_PATH, shell=True)

    @classmethod
    def tearDownClass(cls):
        subprocess.check_output("regsam \"%s\" /unregister" % REG_PATH, shell=True)

I have looked for some examples of using regasm, but they all use .dll files like the ones in MS documentation's Examples.

regasm myTest.dll

@junkmd
Copy link
Collaborator

junkmd commented Nov 20, 2022

To modify code as @matthiasfinger 's suggestion, test_excel.Test_LateBind will be failed.

...\comtypes> python -m unittest comtypes.test.test_excel.Test_LateBind.test -vv
test (comtypes.test.test_excel.Test_LateBind.test) ... FAIL

======================================================================
FAIL: test (comtypes.test.test_excel.Test_LateBind.test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "...\comtypes\comtypes\test\test_excel.py", line 62, in test
    self.assertEqual(xl.Range["A1:C3"].Value(),
AssertionError: Tuples differ: ((10.0, 20.0, 31.4), ('x', 'y', 'z'), ('3', '2', '1')) != ((10.0, 20.0, 31.4), ('x', 'y', 'z'), (3.0, 2.0, 1.0))

First differing element 2:
('3', '2', '1')
(3.0, 2.0, 1.0)

- ((10.0, 20.0, 31.4), ('x', 'y', 'z'), ('3', '2', '1'))
?                                        - ^  - ^  - ^

+ ((10.0, 20.0, 31.4), ('x', 'y', 'z'), (3.0, 2.0, 1.0))
?                                         ^^   ^^   ^^


----------------------------------------------------------------------
Ran 1 test in 3.529s

FAILED (failures=1)

Since the property is set to an array of strings, I don't see a problem with a string being returned when a get is done.
Nevertheless, this breaks backward compatibility and will be something to keep in our mind when upgrading.

And even after applying this change, test_excel.Test_EarlyBind still fails as reported by @karpierz in #212.

@matthiasfinger
Copy link
Author

Would be possible to check whether everything is convertible to a number. But the question for me is: what would be the correct and expected behaviour? Due to the fact that a single value is sent as BSTR, I would expect the same for a tuple, isnt't it?

@junkmd
Copy link
Collaborator

junkmd commented Nov 21, 2022

@matthiasfinger

It seems to be still necessary to prepare its own dll or tlb for testing to check the expected behavior for Variant arrays.

I think that Excel cell values may be inappropriate to use for testing Variant arrays because they rely on number format.
test.test_excel is necessary, but we need to consider what asserting.

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

No branches or pull requests

3 participants