test_sources_update triggers flaky code or is a flaky test

[sten0]

My desktop and laptop aren't fast enough to reproduce the issue locally, but test_sources_update "regularly fails" on half of Debian's CI, reproducibility, and regression testing networks. The Release Team now considers this a serious (release critical) issue. https://bugs.debian.org/1086524

Here is a recent log for 0.10.0-beta1 on arm64:
https://tests.reproducible-builds.org/debian/logs/experimental/arm64/vorta_0.10.0~beta1-1~exp1.build2.log.gz

The dashboard (with logs) for all Reproducibility Builds is available here:
https://tests.reproducible-builds.org/debian/rb-pkg/experimental/arm64/vorta.html

The main dashboard for the CI network is here:
https://ci.debian.net/packages/v/vorta/

Please note that you actually need to click on a particular version under one of the four columns in order to access the CI and regression-testing history for that suite. The nature of the problem Paul Gevers reported is logged on each arch under "testing"; this is where we prepare for the next stable Debian release. Unstable tracks the latest stable version and/or the latest version believed to not have any release critical issues (ie crashes, data loss). Attempts to migrate the experimental version to unstable are also logged here.

All details about environment are also logged; please let me know if you have any questions about interpreting them.

[m3nu]

Hard to address this, if it's not reproducible and the error doesn't happen in our code. I see the test does some patching with Qt widgets, that may cause the crashes elsewhere.

I'll remove those tests for now, until someone has time to redo them.

Since most feedback for 0.10 beta is also covered, I'll do a release for it shortly.

[sten0]

Manu ***@***.***> writes:

Hard to address this, if it's not reproducible and the error doesn't happen in our code. I see the test does some patching with Qt widgets, that may cause the crashes elsewhere.

I agree it's hard to address, and yes, I also believe that the trigger is that monkeypatching.

I'll remove those tests for now, until someone has time to redo them.

Sounds good to me :) It may also be worth forwarding that example upstream as a reproducer, because Trolltech has even more extensive CI than Debian, and they're the most likely to be able to identify the cause if it's on their side.

Since most feedback for 0.10 beta is also covered, I'll do a release for it shortly.

Nice! :) I'll package it without delay. Best, Nicholas

[m3nu]

Upon looking into it more, the error in the logs would point to QThreads fetching file stats that are still running after the test. We take some care in ending those QThreads when they run as background task, but not when refreshing the stats of source folders. So this likely triggers the error. Solution is just to mock those threads in the test, so the don't even start or make sure they are done after the test.

I'm adding some details here so this can be addressed soon. For now I've just disabled those tests to get a release out and test this theory in Debian's CI.

tests/unit/test_source.py::test_valid_and_invalid_source_paths[file:///build/reproducible-path/vorta-0.10.0~beta1/.pybuild/cpython3_3.12_vorta/build/tests/unit/test_source.py/build/reproducible-path/vorta-0.10.0~beta1/.pybuild/cpython3_3.12_vorta/build/tests/unit/test_source.py-False] PASSED
tests/unit/test_source.py::test_sources_update QThread: <<<Destroyed while thread is still running>>>
Fatal Python error: Aborted

Thread 0x0000ffff8efdf180 (most recent call first):
  <no Python frame>

Thread 0x0000ffff8dbcf180 (most recent call first):
  <no Python frame>

[sten0]

Manu ***@***.***> writes:

Upon looking into it more, the error in the logs would point to QThreads fetching file stats that are still running after the test. We take some care in ending those QThreads when they run as background task, but not when refreshing the stats of source folders. So this likely triggers the error. Solution is just to mock those threads in the test, so the don't even start or make sure they are done after the test.

Does this also affect the case when a user "refresh[es] the stats of the source folders"?

I'm adding some details here so this can be addressed soon. For now I've just disabled those tests to get a release out and test this theory in Debian's CI.

Sounds good to me. 0.10.1 is now the latest version in Debian (uploaded directly to sid/unstable).

[sten0]

Hi @m3nu, could you take a look at my previous message when you have a moment?

[m3nu]

Apologies. I didn't see the sentence in the middle due to formatting of the email.

And yes, the source folder tests mostly test getting stats for source folders using QThreads. So the UI isn't blocked, since this can take a long time. My theory would be that some of those QThreads survive after the test and cause the crashes you see in some situations.

For now I've disabled those tests. Best would be to mock the background threads and only test the function they use directly. I assume we partly do this already.