Fix DispatchWithEvents and WithEvents throw on 2nd call

[Avasam]

Fixes #2489

Obligatory "this would've been caught by pyright reportGeneralTypeIssues" (but there's still like a thousand of those, so I'm far from enabling it)

Easier to review by hiding whitespaces.

---

Added tests to cover the case where it's already been dispatched.

But as we found out in #2467 , these tests aren't run on the CI, and I'm unable to run it locally either:

python .\com\win32com\test\testPyComTest.py
Traceback (most recent call last):
  File "E:\Users\Avasam\Documents\Git\pywin32\com\win32com\test\testPyComTest.py", line 12, in <module>
    import pythoncom
ModuleNotFoundError: No module named 'pythoncom'

[Avasam]

In hindsight, this is already tested by the lines above. So it's really just that the test doesn't run on CI...

Suggested change

	# Ensure that if it has already been dispatched, the base classes are the same.
	o2_again = win32com.client.DispatchWithEvents(o, RandomEventHandler)
	assert o2._obj_.__class__.__bases__ == o2_again._obj_.__class__.__bases__
	handler_again = win32com.client.WithEvents(o, RandomEventHandler)
	assert handler.__class__.__bases__ == handler_again.__class__.__bases__

[mhammond]

ModuleNotFoundError: No module named 'pythoncom'

That seems bad.

But as we found out in #2467 , these tests aren't run on the CI, and I'm unable to run it locally either:

huh - I thought that issue was more about the fact that the shell COM tests were not running on CI - is this suggesting no com tests are running on CI at all? We should certainly be able to run some.

[Avasam]

is this suggesting no com tests are running on CI at all? We should certainly be able to run some.

I haven't yet really looked into it, but I added some tests without fixing the issue here: Avasam#7
As we can see, the CI still passes. (I would even have expected the existing tests to also fail).

---

About not being able to run the tests locally, I deleted my venv, re-installed pywin32 globally, opened pwsh as admin, run the postinstall script, then tried running the test directly from the test folder. Now I get:

PS C:\Users\Avasam\AppData\Local\Programs\Python\Python310\Lib\site-packages\win32com\test> python testPyComTest.py
Registration command was:
C:\Users\Avasam\AppData\Local\Programs\Python\Python310\python.exe "C:\Users\Avasam\AppData\Local\Programs\Python\Python310\Lib\site-packages\win32com\test\..\servers\test_pycomtest.py" --unattended > nul 2>&1
Traceback (most recent call last):
  File "C:\Users\Avasam\AppData\Local\Programs\Python\Python310\Lib\site-packages\win32com\test\testPyComTest.py", line 25, in <module>
    RegisterPythonServer(
  File "C:\Users\Avasam\AppData\Local\Programs\Python\Python310\lib\site-packages\win32com\test\util.py", line 98, in RegisterPythonServer
    raise RuntimeError("Registration of engine '%s' failed" % filename)
RuntimeError: Registration of engine 'C:\Users\Avasam\AppData\Local\Programs\Python\Python310\Lib\site-packages\win32com\test\..\servers\test_pycomtest.py' failed

Edit: Drilling down into where exactly it's failing for me, and it's basically this line: gencache.EnsureModule("{6BCDCB60-5605-11D0-AE5F-CADD4C000000}", 0, 1, 1), (which is both in com/win32com/test/testPyComTest.py and com/win32com/servers/test_pycomtest.py). There's also this text that seems... unhelpful:

**** PyCOMTest is not installed ***
  PyCOMTest is a Python test specific COM client and server.
  It is likely this server is not installed on this machine.
  To install the server, you must get the win32com sources
  and build it using MS Visual C++

is com/win32com/src/ not built when running pip install . ?

[Avasam]

About the reason the CI doesn't run these tests, I think it's because PyCOMTest is a level 2 test

pywin32/com/win32com/test/testall.py

Lines 179 to 191 in 03e65bc

	custom_test_cases = [
	# Level 1 tests.
	[],
	# Level 2 tests.
	[
	PyCOMTest,
	PippoTest,
	],
	# Level 3 tests
	[],
	# Level 4 tests.
	[],
	]

whilst only level 1 tests are run on the CI

pywin32/win32/scripts/pywin32_testall.py

Lines 87 to 93 in 03e65bc

	# win32com
	maybes = [
	os.path.join(directory, "win32com", "test", "testall.py")
	for directory in [os.path.join(project_root, "com")] + site_packages
	]
	extras = remains + ["1"] # only run "level 1" tests in CI
	find_and_run(maybes, extras)

[mhammond]

To install the server, you must get the win32com sources
and build it using MS Visual C++

Yeah, that's frustrating :( We should probably work out how to build it in CI.

only run "level 1" tests in CI

That seems like a decision which aged poorly :)

[Avasam]

I opened #2492 to track automated testing. At least this PR can be merged on it's own. I manually tested the MRE from the original issue (which is just a copy-paste of the docstring): just run the same DispatchWithEvents or WithEvents twice. After running even once it's persistant through pywin32 re-installs.

[mhammond]

The indentation of this line looks wrong now there are other entries - should it be dedented?

[Avasam]

I meant for it to be a precision. But I could rewrite as "2 entries, added by the same changeset" like this:

Suggested change

	* Restores many IIDs in `win32com(ext).shell.shell`. See #2486 for details.
	* Restored many IIDs in `win32com(ext).shell.shell`. See #2486 for details. (#2487, @Avasam)

[mhammond]

That "sub item" is more important imo - so maybe it should also move up and note it's a regression? code using win32com.shell is likely broken, right?

[Avasam]

Feel free to reorganize the changelog to your likings if you want keep more important stuff at the top. You have the final say on those before releases ^^

[Avasam]

Explanation for moving this check only in the else branch:
It's not possible for disp_class to be None after the if branch.

[mhammond]

thanks - looks good with a change to the log

[mhammond]

Obligatory "this would've been caught by pyright reportGeneralTypeIssues" (but there's still like a thousand of those, so I'm far from enabling it)

We probably need a way to either incrementally add support for this, or give up on the idea - I'm not going to want to change a ton of code with the possibility of regressions just to possibly avoid further future regressions. Is there literally no way we could have added type annotations and checking to just the code that was changed here, or would we have needed to also touch other unrelated code at the same time?

[Avasam]

Is there literally no way we could have added type annotations and checking to just the code that was changed here, or would we have needed to also touch other unrelated code at the same time?

With mypy I'm adding checking incrementally by having mypy only check annotated functions (by default mypy only check functions it considers typed, and a method is considered typed when it's fully annotated).
Mypy is also very easy to configure per-module.

For pyright, it's not as flexible to decide what's checked. I can disable a check globally (or make it a warning). Or disable in specific files/modules/folders. Which is what I've done, and some checks will only require a few fixes to be re-enabled. Some may never be enabled either because of code that's too dynamic, or because there's too many violations.
Tbh reportGeneralTypeIssues is one of those later ones, as it's pyright's "lump everything that doesn't have a specific code in there" ^^

I'd like to see the public python API typed, and tested. (demos and tests files are also great to test the usage of the type annotations). Fixing stuff along the way.
I'm leaving the C-extensions' type stubs to typeshed. Until SWIG supports generating stubs from the same comments that are used to generate docs, which I've seen is something that might be coming in the future.

[mhammond]

For pyright, it's not as flexible to decide what's checked.

Maybe we should give up on adding more pyright support for now? While I'm generally supportive of adding type annotations etc, I'm not in favour of changing code just to keep these checkers happy unless that code is actually being changed for some other good reason. IOW, changing code just to keep the checkers happy has more risks than benefits IMO.

I'd like to see the public python API typed, and tested. (demos and tests files are also great to test the usage of the type annotations). Fixing stuff along the way.

Similarly, I fully support fixing actual problems, but not so much for fixing theoretical problems. The fact a type checker says 15 year old code might have a problem is only interesting to me when we can identify a case where it is actually a problem. eg, maybe such a checker could indicate a place where a new test makes sense that demonstrates the problem. But changing code which is working fine just because some tool isn't smart enough to know what's a real problem vs which is not doesn't appeal and is a great path to regressions.

I understand things are more subtle than I'm making them sound, but it's probably valuable to make my thinking clear around some of these proposed changes