PR#4: Properly exclude failing tests - rpms/calibre

rpms / calibre

#4 Properly exclude failing tests

Merged 2 years ago by zbyszek. Opened 2 years ago by aimylios.

rpms/ aimylios/calibre exclude-tests into master

Properly exclude failing tests

Marcus A. Romer • 2 years ago

08d1050

497810f8adb992bfecf04e8eacf4ac1340ee6fe0.patch

file removed

-36

		`@@ -1,36 +0,0 @@`
		`- From 497810f8adb992bfecf04e8eacf4ac1340ee6fe0 Mon Sep 17 00:00:00 2001`
		`- From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>`
		`- Date: Tue, 20 Aug 2019 08:50:58 +0200`
		`- Subject: [PATCH] tests: skip unrar test gracefully if module is missing`
		`-`
		`- Fedora (and probably some other distributions) cannot include unrardll`
		`- because of licensing reasons. Unfortuantely this situation is unlikely`
		`- to change in the near future. Skip the test if the module is not`
		`- available.`
		`- ---`
		`- src/calibre/test_build.py \| 5 +++++`
		`- 1 file changed, 5 insertions(+)`
		`-`
		`- diff --git a/src/calibre/test_build.py b/src/calibre/test_build.py`
		`- index c17a5ef785..7514d05046 100644`
		`- --- a/src/calibre/test_build.py`
		`- +++ b/src/calibre/test_build.py`
		`- @@ -17,6 +17,10 @@`
		`-`
		`- is_ci = os.environ.get('CI', '').lower() == 'true'`
		`-`
		`- +try:`
		`- + import unrardll`
		`- +except ModuleNotFoundError:`
		`- + unrardll = None`
		`-`
		`- class BuildTest(unittest.TestCase):`
		`-`
		`- @@ -236,6 +240,7 @@ def test_file_dialog_helper(self):`
		`- from calibre.gui2.win_file_dialogs import test`
		`- test()`
		`-`
		`- + @unittest.skipUnless(unrardll, 'Module unrardll is missing')`
		`- def test_unrar(self):`
		`- from calibre.utils.unrar import test_basic`
		`- test_basic()`

calibre.spec

file modified

+19 -11

		`@@ -6,7 +6,7 @@`

		`Name: calibre`
		`Version: 4.11.2`
		`- Release: 2%{?dist}`
		`+ Release: 3%{?dist}`
		`Summary: E-book converter and library manager`
		`License: GPLv3`
		`URL: https://calibre-ebook.com/`
		`@@ -30,8 +30,6 @@`
		`Patch3: calibre-nodisplay.patch`

		`# Patches that are not suitable for upstream:`
		`- # skip unrardll tests if unrardll has been removed.`
		`- Patch4: https://github.com/keszybz/calibre/commit/497810f8adb992bfecf04e8eacf4ac1340ee6fe0.patch`
		`# sgml was removed, so disable test for it.`
		`Patch5: https://github.com/keszybz/calibre/commit/01bf854923741bf8d6a6328f17d61e0ec5ac3c9f.patch`

		`@@ -171,11 +169,6 @@`
		`# remove bundled MathJax`
		`rm -rvf resources/mathjax`

		`- # Skip tests that require removed fonts`
		`- sed -r -i 's/\b(test_actual_case\|test_clone\|test_file_add\|test_file_removal\|test_file_rename\|test_folder_type_map_case\|test_merge_file)\b/_skipped_\1/' src/calibre/ebooks/oeb/polish/tests/container.py`
		`- # Skip test that fails in mock`
		`- sed -r -i 's/\btest_bonjour\b/_skipped_\0/' src/calibre/srv/tests/loop.py`
		`-`
		`%build`
		`# unbundle MathJax`
		`CALIBRE_PY3_PORT=1 \`
		`@@ -338,10 +331,22 @@`
		`mv %{buildroot}%{_datadir}/calibre/mathjax %{buildroot}%{_datadir}/calibre/mathjax-fedora`

		`%check`
		`- # ignore tests on 32 bit arches for now as there's a pdf issue`
		`- CALIBRE_PY3_PORT=1 python3 setup.py test \`
		`+ # skip some tests which are failing because of missing dependencies (unrar),`
		`+ # problems in mock (bonjour), or they require removed fonts`
		`+ # skip qt test on 32 bit arches for now as there's a pdf issue`
		`+ CALIBRE_PY3_PORT=1 \`
		`+ %{__python3} setup.py test \`
		`+ --exclude-test-name unrar \`
		`+ --exclude-test-name bonjour \`
		`+ --exclude-test-name actual_case \`
		`+ --exclude-test-name clone \`
		`+ --exclude-test-name file_add \`
		`+ --exclude-test-name file_removal \`
		`+ --exclude-test-name file_rename \`
		`+ --exclude-test-name folder_type_map_case \`
		`+ --exclude-test-name merge_file \`
		`%ifarch i686 armv7hl`
		`- \|\| :`
		`+ --exclude-test-name qt`
		`%endif`

		`appstream-util validate-relax --nonet %{buildroot}%{_datadir}/metainfo/calibre-gui.appdata.xml`
		`@@ -391,6 +396,9 @@`
		`%{_datadir}/metainfo/*.appdata.xml`

		`%changelog`
		`+ * Sun Mar 01 2020 Marcus A. Romer <aimylios@gmx.de> - 4.11.2-3`
		`+ - Properly exclude failing tests.`
		`+`
		`* Tue Feb 25 2020 Kevin Fenzi <kevin@scrye.com> - 4.11.2-2`
		`- Add requires on udisks2. Fixes bug #1806362`

aimylios commented 2 years ago

As of version 4.9.0, Calibre allows to specify which test groups and individual tests shall be excluded:
https://github.com/kovidgoyal/calibre/commit/5c795352c9a49249149bfa41caf34c21ad5b0e1a

I suggest to make use of this mechanism, because it is a lot cleaner than having dedicated patches to remove tests or having commands in the SPEC file to modify the source code.

Patch4 can then be removed. Patch5 has to be kept for now, as it only removes a part of a test, but it can be dropped as soon as Calibre 5 is released (which will include a copy of sgmllib).

And instead of completely ignoring the test results on 32-bit architectures, it is probably better to just exclude the problematic test.

The RPM and build logs for F32 and Rawhide based on this PR are available in my private Copr repository:
https://copr.fedorainfracloud.org/coprs/aimylios/calibre-python3/build/1277757/

zbyszek commented 2 years ago

I think the patch for missing unrardll could be upstreamed... Not sure why it wasn't submitted back then. Maybe just because of lack of time. We want to have a solution that also affects the installed system.

But this approach seems cleaner overall, so let's merge.

Pull-Request has been merged by zbyszek

2 years ago

aimylios commented 2 years ago

@zbyszek, you submitted the patch upstream, but it got rejected:
https://github.com/kovidgoyal/calibre/pull/1032

It's true that it would be good to have a patch that completely disables all parts of Calibre which need unrardll. On the other hand, users who need it will most likely install the missing dependencies via RPM Fusion and pip themselves.