Modernising the dev tooling
bennyrowland opened this issue ยท 16 comments
Hello comtypes
ers,
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...
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.
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.
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.
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 black
en 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.
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 themaster
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.
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
becomesmain
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.
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.
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 codeblack
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.
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.
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.
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 differenttox
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?
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.
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.
Lines 135 to 146 in d1f5cd7
Lines 158 to 164 in d1f5cd7
we could use
scikit-build-core
to take care of that. I will investigate a little bit withtox
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
.
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