Integrate PyCOMTest server dll into CI workflow

[geppi]

This pull request addresses issue #2492 and does in a first step add a Visual Studio solution file and a project file, to enable the building and registration of the PyCOMTest server dll with a recent compiler.

The configuration file for the github CI workflow was modified to perform the compilation of the PyCOMTest server dll before running the unit tests.

---

closes #2492, closes #1870

[geppi]

OK, the problem was that the windows-2019 runner does not have the MSVC v143 build tools and the latest Win SDK.
I did retarget the solution and now it builds in the CI workflow.

However, building it locally with Visual Studio 2022 does not work anymore because even with the v142 tools installed it's missing some ATL header files. So the checked in Visual Studio project file is in fact for Visual Studio 2019 and to build with Visual Studio 2022 locally it needs to be retargeted back to v143.

[geppi]

I'm so stupid. How could I forget to register the dll with regsvr32 after building it?
So finally the PyCOMTest server dll gets built and registered in the CI workflow.

Now for the next problem.
The tests fail because of a StringTest.

To be honest, I encountered the same problem when I ran the tests locally but didn't care because it had nothing to do with the COM Record stuff on which I was working at that time. Therefore I simply commented the StringTest out.

The problem is caused by a unicode character.

@Avasam @mhammond Any idea what could cause the issue?

At least we're a step further and the PyCOMTest runs in the CI workflow.

[Avasam]

I think PyCOMTest.idl has the actual character � rather than ® rendered as �

And if you fix that, I'm also pretty sure at line 29 it's supposed to be a ©

I was already piling up typo fixes, so I created a PR and included replacing � with actual characters #2495

[geppi]

@Avasam Thank you for the suggestion.

However, the result is still negative and I'm at my wits end.
While changing the character in the IDL file seems to have fixed what gets returned by the COM server, I have no idea what's screwing up the Python string:

AssertionError: Constant value wrong for StringTest - got Hello Wo®ld, wanted Hello Wo�ld

[geppi]

We're obviously dealing here with a character encoding mess.

After a little more testing I can say that there's not just a difference between the string returned from the COM server and the Python string but also a problem with the character encoding in the display of the github CI workflow output.

The Python string which is reported as Hello Wo�ld in the AssertionErorr output is definitely a proper Python unicode string that contains 'Hello Wo®ld' with the registered trademark sign as the 'r'.

The string returned by the COM server is still wrong, although I have no idea which character encoding the github CI workflow output is using to render this as the plain registered trademark sign. On my local machine it's rendered as 'Hello Wo®ld', i.e. with a capital A avec accent circonfelxe before the registered trademark sign.

To get a better idea what is returned by the COM server and how that compares to the expected Python string, I added some code to encode both strings to 'utf-32' with the following result:

print([ int(c) for c in pyConst.encode('utf-32')])
        [255, 254, 0, 0, 72, 0, 0, 0, 101, 0, 0, 0, 108, 0, 0, 0, 108, 0, 0, 0, 111, 0, 0, 0, 32, 0, 0, 0, 87, 0, 0, 0, 111, 0, 0, 0, 174, 0, 0, 0, 108, 0, 0, 0, 100, 0, 0, 0]

print([ int(c) for c in comConst.encode('utf-32')])
        [255, 254, 0, 0, 72, 0, 0, 0, 101, 0, 0, 0, 108, 0, 0, 0, 108, 0, 0, 0, 111, 0, 0, 0, 32, 0, 0, 0, 87, 0, 0, 0, 111, 0, 0, 0, 194, 0, 0, 0, 174, 0, 0, 0, 108, 0, 0, 0, 100, 0, 0, 0]
                                                                 this is the difference--------------------------------------------->|============|

So the problem is still in the IDL file.
I don't have a lot of experience with COM but to me it looks like the problem is caused by the definition of StringTest as a LPWSTR. Skimming a little through my ancient COM literature it looks like this is calling for trouble.

However, I don't want to spend more time on this issue. So if nobody else knows a remedy this will get stuck here. :-(
@mhammond I'm just wondering how this test could have ever worked?

[geppi]

Finally I found the way to fix the issue with the registered trademark sign.

Investigation of the IDL file with a hex-editor showed that there was the byte sequence 0xC2, 0xAE for the utf-8 encoded registered trademark sign. I changed my editor to save the file ANSI encoded instead of utf-8 so that now there's just 0xAE for the registered trademark sign. This makes the StringTest pass successfully. So it looks like the IDL file needs to be saved ANSI encoded.

Unfortunately PyCOMTest now still fails a little further down when trying to get the CoPyComTest class by CLSID.

This does not happen when I run it locally.

[Avasam]

Locally I fail at

Traceback (most recent call last):
  File "E:\Users\Avasam\Documents\Git\pywin32\com\win32com\test\testPyComTest.py", line 899, in testGenerated
    TestGenerated()
  File "E:\Users\Avasam\Documents\Git\pywin32\com\win32com\test\testPyComTest.py", line 503, in TestGenerated
    TestCounter(coclass, True)
  File "E:\Users\Avasam\Documents\Git\pywin32\com\win32com\test\testPyComTest.py", line 723, in TestCounter
    num = int(random.random() * len(counter))
TypeError: object of type 'CoSimpleCounter' has no len()

---

But other than that I can also confirm everything else passes locally. So there's something different with the CI.

Traceback (most recent call last):
File "D:\a\pywin32\pywin32\com\win32com\test\testPyComTest.py", line 902, in testGenerated
TestGenerated()
File "D:\a\pywin32\pywin32\com\win32com\test\testPyComTest.py", line 498, in TestGenerated
coclass_o = GetClass("{8EE0C520-5605-11D0-AE5F-CADD4C000000}")()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\runneradmin\AppData\Roaming\Python\Python312\site-packages\win32com\client\CLSIDToClass.py", line 52, in GetClass
return mapCLSIDToClass[clsid]
~~~~~~~~~~~~~~~^^^^^^^
KeyError: '{8EE0C520-5605-11D0-AE5F-CADD4C000000}'

Could take a look at the generated python from genpy

[mhammond]

Far out, what a mess :( So the udl compiler assumes a specific encoding?

[Avasam]

Could take a look at the generated python from genpy

In https://github.com/Avasam/pywin32/tree/refs/heads/testing-for-PyCOMTest_server I hackily made the CI upload the gen_py folder content as an artifact.
Here's my local successful testPyComTest.py run on Python 3.10.11: gen_py Python 3.10.11.zip

Here's the CI's pywin32_testall on Python 3.10: gen_py-3.10.zip
All artifacts: https://github.com/Avasam/pywin32/actions/runs/13890314811

Edit: I ran python win32/scripts/pywin32_testall.py -v -skip-adodbapi locally and it failed as well ! (on the same KeyError: '{8EE0C520-5605-11D0-AE5F-CADD4C000000}'

You can see that pywin32_testall creates a package (fails) whilst testPyComTest creates a module (succeeds)

[Avasam]

Alright, narrowing down the issue to being that when run under pywin32_testall, the generated module is never imported, hence the CLSID is missing from mapCLSIDToClass

>>> from win32com.client.CLSIDToClass import *
>>> GetClass("{8EE0C520-5605-11D0-AE5F-CADD4C000000}")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\Avasam\AppData\Local\Programs\Python\Python310\lib\site-packages\win32com\client\CLSIDToClass.py", line 52, in GetClass
    return mapCLSIDToClass[clsid]
KeyError: '{8EE0C520-5605-11D0-AE5F-CADD4C000000}'
>>> import importlib
>>> importlib.import_module('win32com.gen_py.6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1.CoPyCOMTest')
<module 'win32com.gen_py.6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1.CoPyCOMTest' from 'C:\\Users\\Avasam\\AppData\\Local\\Temp\\gen_py\\3.10\\6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1\\CoPyCOMTest.py'>
>>> GetClass("{8EE0C520-5605-11D0-AE5F-CADD4C000000}")
<class 'win32com.gen_py.6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1.CoPyCOMTest.CoPyCOMTest'>

testPyComTest.py doing gencache.EnsureModule("{6BCDCB60-5605-11D0-AE5F-CADD4C000000}", 0, 1, 1) generates the standalone module.

But when it already exists as a package, only 6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1.__init__ is imported! but the code to add to mapCLSIDToClass is found in 6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1.CoPyCOMTest

[geppi]

@Avasam I'm a little confused by your findings because I don't understand where in the code the PyCOMTestLib module is generated as a package. Is the 6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1 package generated anywhere before the call to gencache.EnsureModule("{6BCDCB60-5605-11D0-AE5F-CADD4C000000}", 0, 1, 1) in testPyComTest.py?

As far as I understand the way to force the creation of a package is to pass the bForDemand=True optional parameter to gencache.EnsureModule. However, in the code the dafault value for this parameter is False and I don't see it redefined anywhere.

[Avasam]

@Avasam I'm a little confused by your findings because I don't understand where in the code the PyCOMTestLib module is generated as a package.

Me neither. I dig and found the answer, see below.

Is the 6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1 package generated anywhere before the call to gencache.EnsureModule("{6BCDCB60-5605-11D0-AE5F-CADD4C000000}", 0, 1, 1) in testPyComTest.py?

It would seem like it, since with a 6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1 folder/package, gencache.EnsureModule won't to anything (it stays a package). And as you also described, the call in PyCOMTest generates a module, not a package.

Oh also if the module 6BCDCB60-5605-11D0-AE5F-CADD4C000000x0x1x1.py already exists, it'll get replaced by the package version.

---

I ended up bissecting the tests, first found the culprit was part of the .\com\win32com\test\testall.py test suite.
Then found it's simply com/win32com/test/testArrays.py and only that test that's causing our current issue.
Specifically, this line:

pywin32/com/win32com/test/testArrays.py

Line 54 in c0f06cf

self.arr = gencache.EnsureDispatch("PyCOMTest.ArrayTest")

Adding bForDemand=False to EnsureDispatch there seems to work. But that does feel like a patch without understanding the core issue:

1. Why EnsureDispatch with bForDemand=True there?
2. Is it expected that generating packages in gen_py, the __init__ doesn't import its submodules, causing the various inner RegisterCLSID and RegisterCLSIDsFromDict to never be called ?
3. Should these tests even affect each other ?

[geppi]

Currently something is wrong with the CI workflow.
The build process fails for all Python versions except 3.8 with:

win32/src/PythonService.cpp(49): fatal error C1083: Cannot open include file: 'PythonServiceMessages.h': No such file or directory

[geppi]

Back to the main topic although we can't do much until the issue with the CI pipeline is solved.

Thank you @Avasam for taking the effort to bisect the tests.

1. Why EnsureDispatch with bForDemand=True there?

Good question, especially because everywhere else in genchache.py the default value is taken from the global bForDemandDefault=0. Only EnsureDispatch sets it locally to True which makes the name of the global kind of a misnomer.
However, your change in the last commit fixes the issue, as can be seen in the only CI run that accomplished a successful build, i.e. CI / Build and test (3.8, x64).
Nevertheless, it fails a little later in the code and this will be the next issue to solve, although according to my local test runs the last one. :-)

Is it expected that generating packages in gen_py, the init doesn't import its submodules, causing the various inner RegisterCLSID and RegisterCLSIDsFromDict to never be called ?

If I remember correctly, somewhere in the code is a comment that suggests, that the main reason for implementing the package stuff was to speed up the process for large type libraries by creating only those interface modules of a component that are actually required. I would therefore expect that it shouldn't affect functionality whether you set bForDemand as False or True but maybe I'm missing something. @mhammond What do you think?

Should these tests even affect each other ?

No.

[Avasam]

Wooh! Progress.

The non 3.8 CI failures are unrelated and something I noticed as well in a different PR. I'll have to investigate separately.

The CoSimpleCounter test failure I'm also getting locally as mentioned in #2493 (comment)
I may have an idea about that one. But probably won't have time to look into it until next week.

[mhammond]

1. Why EnsureDispatch with bForDemand=True there?

Probably just to test bForDemand. IIRC, that flag is intended to generate just enough from a typelib that actual interfaces etc within the typelib are generated on demand - so it's not necessary to wait for everything in the typelib to have generated support before the object you care about can be used. It only makes sense in massive typelibs, like office etc. It may not make sense any more.

If I remember correctly, somewhere in the code is a comment that suggests, that the main reason for implementing the package stuff was to speed up the process for large type libraries by creating only those interface modules of a component that are actually required. I would therefore expect that it shouldn't affect functionality whether you set bForDemand as False or True but maybe I'm missing something. @mhammond What do you think?

Right, that sounds correct - the intent was bForDemand would not change the observed behaviour (although it would change when stuff is generated - either at the very start all at once, or progressively as the code starts using different objects).

I'll try and re-read some of the above soon, but let me know if there are other specific problems here (it sounds like you might have got past this though?)

win32/src/PythonService.cpp(49): fatal error C1083: Cannot open include file: 'PythonServiceMessages.h': No such file or directory

Not sure if this is still blocking you, but this is generated from the .mc file - iirc, distutils etc handled this, although there is an ominous comment here

[Avasam]

I fixed the unrelated CI build failure in #2498

[geppi]

Finally this is good to go.

The last commit will also close #1870 which did in the last comment already provide the solution for the CoSimpleCounter test failure.

[Avasam]

The last commit will also close #1870 which did in the last comment already provide the solution for the CoSimpleCounter test failure.

I also recently read that issue and figured it was likely related. But unfortunately was too occupied right now to try the changes myself. (GitHub reviews on the go are easy to do ^^)

Thanks a lot for taking your time to resolve all this!

[Avasam]

I'd like to see the build script in .github/workflows/main.yml moved to a local script file that can easily be run locally as well. And added to https://github.com/mhammond/pywin32/tree/main/com/win32com/test/readme.txt 's documentation. But I'm perfectly happy seeing this merged as-is and making said changes myself as a follow-up.

Waiting for @mhammond 's review.

[Avasam]

Would you like to add a change entry for the #1870 fix ?

[mhammond]

Thanks!