Modernising the dev tooling

[bennyrowland]

Hello comtypesers,

As comtypes is currently experiencing a new lease of life (thanks particularly to the efforts of @junkmd), I wanted to gauge the level of interest from contributors in adding in new tooling to support the development process. There is now such a rich ecosystem of packaging tools including things like tox/nox for isolating and automating test builds, formatters like black (that can be run with pre-commit), linters like pylint or ruff, setuptools_scm for version management, and towncrier for generating the changelog.

While I am not suggesting the immediate or wholesale adoption of all these options, I think that incorporating some of these tools could make the development process easier for new contributors to get started with, and probably simplify things for everybody. However, I realise that this would also require a certain amount of work, and also some discussion to try and achieve consensus on what tools to use, and that may not appeal to everyone involved. So I decided to open this issue to see whether I get enthusiastic support, naked hostility, or studied indifference. Let me know what you all think...

[junkmd]

Thank you so much for the compliment, @bennyrowland!

This issue is worth to be pinned (so I actually did).

I will summarize my opinions during my free time this weekend.

[junkmd]

First of all, most of the modern tools you have exemplified are for Python3 only.

So if we were to adopt some of them, it would be the drop_py2 branch, not the master branch which still supports Python 2.7.

[junkmd]

Just IMO

formatters like black

This has already been partially introduced in the drop_py2 branch(#418).
The GHA checks if the PRed code is in the black==22.12.0 format.

I am contributing to typeshed.
In that repository, GHA runs auto-format every PR using black.
I thought that workflow to be excellent.

I tried to introduce it in comtypes.
But it was difficult for bots to add commits with git-auto-commit or pre-commit ci due to permissions.
So we are currently limiting it to style checks only.

If other (perhaps at the Member level) maintainers agree to give these bots/actions commit privileges, I think we can introduce auto-formatting as well.

things like tox/nox for isolating and automating test builds

It is important to be able to test with multiple Python versions.
However, since we are currently able to test multiple versions of Python with AppVeyor CI, I do not think it is necessary to introduce those third parties yet.
AppVeyor CI is tied to the Enthought repository, so it cannot run in the forked repository.
I am thinking that it might be better to migrate it to GHA so that developers can test them in their own forked repositories before PR.

linters like pylint or ruff

There is still some old code in comtypes, so we get a lot of warnings when we apply the linter (from ctypes import * is troublesome!).
On the other hand, it might be reasonable to add stuffs to the configuration files in order not to conflict with the black style.
For example, adding max-line-length = 88 to setup.cfg for flake8.
Regarding coding style, currently, I think it is unnecessary to go beyond what black formats.

setuptools_scm for version management

This may be a good idea because it might simplify the release process.
The distutils are still in setup.py, so we'll have to deal with that(#228).

towncrier for generating the changelog

This may be a good idea because it might simplify the release process.

[bennyrowland]

Thanks, @junkmd, for your thoughts. I would say that adding a pre-commit config file that devs can use to install a commit hook that will run black (and optionally other things) locally before committing would be a good way to make it easier for devs to get their code black compliant before letting it go up to a GHA to do the same thing in the cloud. Optional, but easier for most people than setting it up themselves, or ensuring compliance in another way. Do note though that it is much better to blacken the whole codebase first, because otherwise every code file that people touch will get reformatted by black and their commits will then be a mixture of black formatting and whatever they were actually doing, which makes understanding the change much harder.

To my mind part of the advantage of tox et al. is not just being able to test on multiple Python versions, but to provision an isolated venv for tests and potentially have more complicated test setups that can still be run very easily. In #478 I proposed adding a test time dependency on pyfakefs - using tox would make it easy to add this to the testing venv without affecting the user's main Python install, or even having to worry about it. It would also be possible to e.g. run a registration command on a .tlb file used for testing before running the unittest commands, then unregister it afterwards, without having to make the user type in all the relevant commands themselves each time.

The reference to setup.py made me think of another proposal:

Moving to pyproject.toml

---

How much of the code in setup.py is still relevant? In an age where most people will be installing via wheel anyway, do we need to have the setuptools post-install commands etc. If we move all the metadata into pyproject.toml it will be static, much easier to read, and we can add config for things like black/setuptools_scm/towncrier in there as well.

[bennyrowland]

First of all, most of the modern tools you have exemplified are for Python3 only.

So if we were to adopt some of them, it would be the drop_py2 branch, not the master branch which still supports Python 2.7.

My understanding is that the day drop_py2 becomes main is not too far away now? Of course, if we version bump so that 1.2.X is py3 only and 1.1.X continues to support py2 then there is no reason not to maintain a py2 branch indefinitely if desired.

[junkmd]

I am aware that supporting Python2 will be available up to 1.2.x, and supporting Python3 only will be available from 1.3.0.

the day drop_py2 becomes main is not too far away now?

That perception as well for me.

Actually, I expect that drop_py2 will be merged to master and then master will be renamed to main.

there is no reason not to maintain a py2 branch indefinitely if desired.

I wonder that who would be a contributor or a maintainer to the py2 even if it were left.
I think that few features that can be backported from the main branch to the py2.
I believe it is sufficient to reference 1.2.x from PyPI or GitHub.

[junkmd]

Moving to pyproject.toml

Currently, the AppVeyor CI and the test_pip_install depend on the setup.py.

If these tests can be run equivalently using the pyproject.toml instead of setup.py, I have no objection.

[junkmd]

adding a pre-commit config file that devs can use to install a commit hook that will run black (and optionally other things) locally before committing would be a good way to make it easier for devs to get their code black compliant before letting it go up to a GHA to do the same thing in the cloud

I see.
It looks good to me to automatically codebase formatting by black with commit hooks in the developer's local environment.

The existing GHA would act as the gatekeeper.

It won't be a problem as long as we are careful about maintaining things like requirements-dev.txt and/or config files.

[junkmd]

When implementing #486, I considered that registering TLBs/DLLs for testing in the registry before testing and unregistering them after testing is better than worrying about native COM libraries in the test environments.

If you were to configure tox to handle the registration of DLLs, what configuration file would you write?

Additionally, I believe it would be more preferable to build the TLBs/DLLs through CD/CI from the source code instead of keeping pre-built TLBs/DLLs.

Of course, if these are to be introduced, they will not be done all at once, but will be step by step.

I would appreciate your insights.

[bennyrowland]

I have a project which implements a COM server using comtypes with tox for testing. In that case, I depend on a separate COM server implemented in C++ and in a .dll (in my case installed as a dependency through a separate wheel, but could be built as part of the package instead) and I have something like the following in my tox.ini:

[testenv]
whitelist_externals = cmd
commands =
    cmd /c "C:\\windows\\syswow64\\regsvr32.exe /s {envsitepackagesdir}\\path\\to.dll"
    pytest {posargs:-vv}

There may be cleaner ways of doing this but I haven't figured them out yet. I also haven't bothered unregistering afterwards but tox supports a commands_post (which always runs even if commands throws an error and stops) which could be used to do that. Obviously getting the right regsvr is important, in my case I know I am 32-bit so just hard-coded it but it shouldn't be that hard to get the bit size from the Python executable or set up different tox envs for the different versions. It is probably also something that would suit a plugin quite well.

I did try using a mocked registry to avoid all this stuff, but too much of the stuff accessing the registry is in the COM layer rather than in Python so that didn't work. Another way to approach this would be to do the registering on a per test basis (e.g. using subprocess.run() or similar). This could be wrapped up in a decorator or a fixture to make it easier to manage on an individual test level (including unregistration etc.)

Any testing DLLs/TLBs obviously should not be part of the main package, which means if they are to be built from source then we need some kind of test build process distinct from building the main comtypes package. This issue seems relevant: tox-dev/tox#1162. Basically you would have to run some commands inside the tox run which would build the test components, then install them to wherever they needed to be. This could be just a simple script, but it might be worth setting up a separate Python package to build them in - we could then use scikit-build-core or similar to make sure that Visual Studio or whatever else is required to build them is correctly set up, plus it would be easy to put the resulting objects into known locations, as well as caching build artefacts to speed up builds etc.

[junkmd]

@bennyrowland

Thanks.

I think your examples should be very useful.

Obviously getting the right regsvr is important, in my case I know I am 32-bit so just hard-coded it but it shouldn't be that hard to get the bit size from the Python executable or set up different tox envs for the different versions. It is probably also something that would suit a plugin quite well.

"get the bit size from the Python executable or set up different tox envs for the different versions" is necessary for introducing tox.
It is important to guarantee behavior in both 32-bit and 64-bit.

I did try using a mocked registry to avoid all this stuff, but too much of the stuff accessing the registry is in the COM layer rather than in Python so that didn't work. Another way to approach this would be to do the registering on a per test basis (e.g. using subprocess.run() or similar). This could be wrapped up in a decorator or a fixture to make it easier to manage on an individual test level (including unregistration etc.)

Depending on the developer's permissions, those COM libraries could not be registered. In that case, those tests need to be skipped.
The CI build agent could do something as Administrator. So the CI could run the test that was skipped in the developer's environment.

Any testing DLLs/TLBs obviously should not be part of the main package, which means if they are to be built from source then we need some kind of test build process distinct from building the main comtypes package.

This repository currently contains ... /comtypes/test/mylib.tlb, ... /comtypes/test/TestComServer.tlb and ... /comtypes/test/TestDispServer.tbl.
Is this not a good practice currently?

[bennyrowland]

This repository currently contains ... /comtypes/test/mylib.tlb, ... /comtypes/test/TestComServer.tlb and ... /comtypes/test/TestDispServer.tbl.
Is this not a good practice currently?

Well, personally I would put the test folder outside of the main package (in the repo but not in the built sdist or wheel), I don't see why it should be installed by every user on production systems that don't want to run the tests. But I certainly wouldn't add a load of Visual Studio files like .sln/.vcproj and the .c and .h files used to build test DLLs into that folder (just as the repo currently has the source folder). It is just bloating the installable package with irrelevant files. It would not be difficult to have a build step which would run a compilation step for some test DLLs and install them into the tox venv (potentially reusing the build for envs which could share artefacts). Making sure that the right build tools are available for the running process might be more difficult, but I think we could use scikit-build-core to take care of that. I will investigate a little bit with tox to see what might be achieved, and how easily.

I think we could define a pytest mark to indicate tests that needed admin permissions and have a "skip-if" paradigm so that they would automatically run or not depending on the permissions pytest was run with.

[junkmd]

Thank you for your knowledge.
I only use comtypes as a client, so it is very helpful to have you in the community with your server-side knowledge.

put the test folder outside of the main package (in the repo but not in the built sdist or wheel)

Modern project repositories usually locate tests outside of the package.
Perhaps it is time for comtypes to do the same.
If we do so, we will need to change the code in setup.py and modify CONTRIBUTING.md, which are written under the assumption that the tests are currently placed in the package.

comtypes/setup.py

Lines 135 to 146 in d1f5cd7

	package_data={
	"comtypes.test": [
	"TestComServer.idl",
	"TestComServer.tlb",
	"TestDispServer.idl",
	"TestDispServer.tlb",
	"mytypelib.idl",
	"mylib.idl",
	"mylib.tlb"
	"urlhist.tlb",
	"test_jscript.js",
	]},

comtypes/setup.py

Lines 158 to 164 in d1f5cd7

	packages=[
	"comtypes",
	"comtypes.client",
	"comtypes.server",
	"comtypes.tools",
	"comtypes.test",
	],

we could use scikit-build-core to take care of that. I will investigate a little bit with tox to see what might be achieved, and how easily.

I really appreciate that.

we could define a pytest mark to indicate tests that needed admin permissions and have a "skip-if" paradigm so that they would automatically run or not depending on the permissions pytest was run with.

If it is just skip markers, I think we should just make do with it in unittest as used in the Excel test.
But I also believe pytest is very useful and is one of the things I would like to introduce.
As for pytest, an issue has already been submitted in #123.
However, in context, #123 may have been an issue that should have been closed when #121 was merged.
I will try to redirect the discussion into this issue, since the concerns about introducing pytest should be equivalent to those about introducing a third-party library, such as tox.

[junkmd]

In #121, it was suggested that CI be used for tests including builds.
However, it did not work because of the version of VS.

It should be helpful for us to see what @orf and @cfarrow did in #121 and #123 since we are dealing with similar matters.

[junkmd]

I know that it may raise a Windows fatal exception: access violation when using pytest. This often occurs with pywinauto. It can be suppressed by running pytest with the -p no:faulthandler option.

However, if possible, I would like to avoid using this testing option and avoid making errors "in the right way".

related to:
#204
pywinauto/pywinauto#858
pytest-dev/pytest#5440
https://stackoverflow.com/questions/57523762/pytest-windows-fatal-exception-code-0x8001010d
https://docs.pytest.org/en/stable/changelog.html#pytest-5-0-0-2019-06-28