MeVisLab / pythonqt

Dynamic Python binding for Qt Applications
https://mevislab.github.io/pythonqt/
GNU Lesser General Public License v2.1
237 stars 86 forks source link

CI tests for generator component are missing #138

Open iakov opened 8 months ago

iakov commented 8 months ago

Missing test for generator, but definitely some regression tests are required. I suggest implementing the following simple steps.

  1. Add ASAN_OPTIONS to generation step, at least ASAN_OPTIONS="detect_stack_use_after_return=1:fast_unwind_on_malloc=0"
  2. Change each CI job to have an additional step to build with freshly generated sources.
  3. (Optional, arguable) add step to diff the generated_cpp sources with the currently used generated_cpp_XXX to enforce the regular update.

The last one should be done after the fixes in generator for the known non-determinism.

mrbean-bremen commented 8 months ago

@iakov - are you planning to make a PR for this, or should someone else look at it?

iakov commented 8 months ago

@YuriUfimtsev is "the chosen one". This is the issue for him, he knows, we've discussed this in person.

iakov commented 8 months ago

Partially done and merged: https://github.com/MeVisLab/pythonqt/commit/6d90f6ca8eb4f5008cba0476a7f14f877154298c Next step: add LSan suppressions via LSAN_OPTIONS=suppression=lsan.supp with corresponding lsan.supp if it is too time-consuming to fix current leaks. Useful small step for regression tests. @YuriUfimtsev

iakov commented 8 months ago

Probably, detect_leaks=1 can be turned on (with tiny suppressions for globals) after #148

mrbean-bremen commented 8 months ago

Probably, detect_leaks=1 can be turned on (with tiny suppressions for globals) after https://github.com/MeVisLab/pythonqt/pull/148

Yes, this is the hope... let's wait and see, hopefully that branch will be merged soon.

mrbean-bremen commented 7 months ago

I had switched on detect_leaks=1 in a branch and noticed that there is still a lot of leaks in the generator. Personally, I'm not too worried about this, as the generator is only run on demand every once in a while, and it doesn't bother me if that takes a few MB more of RAM. I don't think it is worth the possibly large effort of fixing this, but if anyone is willing, I won't object 😀

As for the other other goals: there is currently the "Check generated_cpp" build job that runs for Qt 5.12, 5.15 and 6.5, for ubuntu and Windows (MSVC). We also may add Macos builds here, and if needed more versions. We could also change the existing jobs to compile the generated sources, and consolidated the existing tests. Here is a list of the versions currently tested in CI for reference:

Build

ubuntu 20.04  Qt 5.12.8    Python 3.8.10
ubuntu 22.04  Qt 5.15.3    Python 3.10.12
centos 7      Qt 5.9.7     Python 2.7.5
rockylinux 9  Qt 5.15.3    Python 3.9.18
MacOS 11      Qt 5.9.9     Python 3.6.15
MacOS 12      Qt 5.12.12   Python 3.11.6
MinGW 5.3     Qt 5.11.3    Python 3.6.8
MinGW 7.3     Qt 5.12.12   Python 3.10.11
MSVC 2017     Qt 5.11.3    Python 3.6.8

Check generated

MSVC 2022     Qt 5.12.12   Python 3.12.1
MSVC 2022     Qt 5.15.2    Python 3.12.1
MSVC 2022     Qt 6.5.3     Python 3.12.1
ubuntu 22.04  Qt 5.12.12   Python 3.12.1
ubuntu 22.04  Qt 5.15.2    Python 3.12.1
ubuntu 22.04  Qt 6.5.3     Python 3.12.1

As for the last point (compare generated files with checked in generated) - I'm not sure how helpful that would be, given that the checked in generated files are a snapshot of a specific Qt version, and APIs tend to change in Qt also in different patch releases. As I wrote elsewhere, we are now generating the sources ourselves and don't rely on the checked-in code, but I see that it can be helpful to have the sources already checked in.

I'm trying to wrap up a bit before a Qt 5.15 / 6.5 release soon to come, thus my ramblings...

YuriUfimtsev commented 7 months ago

I had switched on detect_leaks=1 in a branch and noticed that there is still a lot of leaks in the generator. Personally, I'm not too worried about this, as the generator is only run on demand every once in a while, and it doesn't bother me if that takes a few MB more of RAM. I don't think it is worth the possibly large effort of fixing this, but if anyone is willing, I won't object 😀

Yes, a bit more refactoring work and I will open PR that decreases almost all memory leaks in the generator. I will do it today or tomorrow :)

he-hesce commented 7 months ago

It seems Rocky Linux 9.3 has Python 3.11 as optional install. Would be cool to have an alternative rockylinyx 9 with Python 3.11 in matrix. We plan to offer support for both Python 3.9 and 3.11 on Rocky Linux 9 in our product.

mrbean-bremen commented 4 weeks ago

@YuriUfimtsev - what's the state here? Is there something left to do? @he-hesce - is that still relevant? If yes, you can always make a PR to adapt the build matrix.

he-hesce commented 4 weeks ago

@mrbean-bremen : Yes, we are still using/supporting both Python 3.9 and Python 3.11 on Rocky Linux 9 as both Python versions are supported with 3.9 being default. Is it just to update the build matrix? Is there an image/container available with Rock9+3.11?

mrbean-bremen commented 4 weeks ago

Is there an image/container available with Rock9+3.11?

I actually have no idea, but the build should be probably similar to the current CentOS build. I have no experience there, so I thought it would be better done by somebody actually knowing the stuff :)