Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip test if fpdf is not available #2408

Closed
kitterma opened this issue Jan 14, 2024 · 8 comments · Fixed by #2419
Closed

Skip test if fpdf is not available #2408

kitterma opened this issue Jan 14, 2024 · 8 comments · Fixed by #2419

Comments

@kitterma
Copy link
Contributor

Trying to run the tests in a constrained environment

Environment

Which environment were you using when you encountered the problem?

$ python -m platform
Linux-5.10.0-27-amd64-x86_64-with-glibc2.31

$ python -c "import pypdf;print(pypdf._debug_versions)"
pypdf==3.17.4, crypt_provider=('pycryptodome', '3.20.0'), PIL=10.1.0

The fpdf module is not available in all environments. It would be nice if the one test that uses is could be skipped if it's not available. See the patch below.

Scott K

diff --git a/tests/test_writer.py b/tests/test_writer.py
index 281232c..22aa8b3 100644
--- a/tests/test_writer.py
+++ b/tests/test_writer.py
@@ -1973,6 +1973,7 @@ REFERENCES 76"""
         bookmarks.append(new_bookmark)


+pytest.importorskip("fpdf")
 def test_merging_many_temporary_files():
     def create_number_pdf(n) -> BytesIO:
         from fpdf import FPDF
@stefan6419846
Copy link
Collaborator

Thanks for the report. Do you want to submit a PR as you already have generated a patch anyway?

kitterma pushed a commit to kitterma/pypdf that referenced this issue Jan 15, 2024
@kitterma
Copy link
Contributor Author

Done. #2412

@kitterma
Copy link
Contributor Author

This change did not get incorporated in the final version of #2412.

I understand that for open environments, fpdf is easily available and so this wouldn't be needed. In Debian we test in a closed environment and fpdf is not available. I'd rather just skip the test than package the module, which I think is a reasonable thing to do in restricted environments. If you don't want the patch upstream, it's no problem to keep it as a Debian only patch.

@MartinThoma
Copy link
Member

In Debian we test in a closed environment

Oh, interesting! I think that is something I didn't/don't understand. Could you please explain that?
Why can you install pypdf and the other dependencies, but not fpdf?

@stefan6419846
Copy link
Collaborator

Probably because fpdf currently is not built by downstream and thus running these tests there does not make much sense from the packaging perspective.

@kitterma
Copy link
Contributor Author

That's correct. In Debian there is a requirement that it be possible to build any package in Debian using only packaged software, so we have packages for things like flit, pytest, and pil. We have a CI environment that runs tests for all packages that are configured to do so. For these tests, they are also constrained to packages from the Debian archive. We don't install software directly from external sources for package build, installation, or test. For fpdf, it's not packaged and it is really not worth the work to do so in order to run one more test.

@MartinThoma
Copy link
Member

Thanks for the explanation!

MartinThoma added a commit that referenced this issue Jan 21, 2024
The Debian ecosystem installs only packages which are also present
in the Debian ecosystem for testing. This includes pytest, but it
does not include fpdf2.

Adding the line 'pytest.importorskip("fpdf")' within the test
and before the import ensures that pytest skips the test in
case fpdf is not installed.

Closes #2408
MartinThoma added a commit that referenced this issue Jan 21, 2024
The Debian ecosystem installs only packages which are also present
in the Debian ecosystem for testing. This includes pytest, but it
does not include fpdf2.

Adding the line 'pytest.importorskip("fpdf")' within the test
and before the import ensures that pytest skips the test in
case fpdf is not installed.

Closes #2408
@kitterma
Copy link
Contributor Author

kitterma commented Jan 21, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants