fix: Unittest poluted with warning about missing Qt translations

[buhtz]

While running unittest warning messages about missing qt translations are now ignored.

@graysky2 : Does this help with ARCH?

Note: I will refactor that whole test and create a check-output-but-ignore-the-rest-function() after the next release.

[aryoda]

@buhtz Please also add a changelog entry 😉

[buhtz]

I will wait with merging because it seems Arch still has problems. I asked for a detailed bug report.

[aryoda]

I will wait with merging because it seems Arch still has problems. I asked for a detailed bug report.

I think the failing unit test on ARCH may (still) happen because this fix is not yet contained. It contains "only":

https://github.com/bit-team/backintime/commit/5202a3e46c134756a5ecd883947fa4d9ed2d557f.patch

See the package build script for this:
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=backintime#n14

I am still racking my brain over how we could improve this bug hunt/fix/repackage/retest cycles but have no idea so far...

[buhtz]

I think the failing unit test on ARCH may be caused because this fix is not yet contained. It contains "only":
https://github.com/bit-team/backintime/commit/5202a3e46c134756a5ecd883947fa4d9ed2d557f.patch

Then we should merge?

I am still racking my brains over how we could improve this bug hunt/fix/repackage/retest cycles but have no idea so far...

Me, too. I have an idea to make the test more robust. But before trying this I will discuss another approach.
Why not just remove all lines beginning with "WARNING" or "Warning"? What do you think?

And another more clean approach:

I try to remember what I've learned from "Khorikov (2020) Unit Testing - Principles, Practices, and Patterns".
Let me think out loud...
What is the "system under test" (SUT) here? What is the behavior that should be tested?
In fact it is an integration test, not a system or unit test. So the rules and goals are a bit different compared to unit tests.

The big questions in this test is if a local backup can be made successfully?
As I stated in the docstring of the test function I wouldn't validate that based on the log output but just on the resulting files in the filesystem. And the latter is really done in the last line of that test.

backintime/common/test/test_backintime.py

Lines 229 to 232 in 37018e0

	subprocess.check_output(["diff",
	"-r",
	"/tmp/test",
	"/tmp/restored"])

You always have to weight the benefit and the cost of a test. Every test has a cost of maintenance, some more, some less.
I would like to propose to remove the whole log-output-checking part of that test. It won't lower the value/benefit of that test. The test still do test the behavior it is intended to test. No problem.

A test, even if it is an integration test, should never try to catch everything. Of course there is a (low!) risk that we might lose some information or cases where problems occur that could be covered by checking the log output on stdout/stderr. But this is not the intention of that test to cover unknown cases.

Remember the last incidences with this test. There is this Qt-translation warning. There was the wayland and dbus messages, etc. Everytime that test failed and cost us time. But it did not failed because the behavior under test was not successful. It failed just because of a side effect it should not cover. No benefit just costs. Let's kill this output checking code. 😈

[aryoda]

Why not just remove all lines beginning with "WARNING" or "Warning"? What do you think?

I am inclined to go this way.

Still I would like to look back which issues could not have been fixed without the unit tests failing due to warnings (esp. AUR user are quite responsive because the installation fails due to unit tests failures; standard-packaged installations do unit testing only once at the maintainer side so user may not even realize that there is a warning that could help us to fix something).

Shall we move this discussion into a new issue ("more robust unit tests")?

[aryoda]

I have downloaded and attached the unit test output, it clearly is caused by the translation warning too:
Unit test log output - TNkXL.txt

WARNING: PyQt was not able to install a translator for language code "C". Deactivate translation and falling back to the source language (English).

https://aur.archlinux.org/pkgbase/backintime#comment-934482

[aryoda]

I think we can merge this PR since it fixes the reported failing unit test.

@buhtz Do you push the changelog entry first before merging?