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

embed_file() fails tests on Windows for text files #515

Closed
gmischler opened this issue Sep 1, 2022 · 5 comments
Closed

embed_file() fails tests on Windows for text files #515

gmischler opened this issue Sep 1, 2022 · 5 comments
Labels

Comments

@gmischler
Copy link
Collaborator

Error details
Those test cases that embed a text file fail, because the data are stored with \n\r line endings there. Since they are read in binary mode, the resulting PDF is different from the stored one.

grafik

Minimal code

$> pylint -k test_embed

Environment

  • Windows
  • fpdf2 HEAD

Not sure if the difference has any practical consequences, or what the best solution would be. Maybe embed_file() needs a flag for reading text files in text mode.

@gmischler gmischler added the bug label Sep 1, 2022
@Lucas-C
Copy link
Member

Lucas-C commented Sep 2, 2022

Thank you for reporting this @gmischler
I'll get a loot at it when I can, probably next week

@Lucas-C
Copy link
Member

Lucas-C commented Sep 5, 2022

I have just tested this under Windows and those tests pass for me:

pytest -k test_embed
==================================================================== test session starts ===================================================================== platform win32 -- Python 3.8.3, pytest-7.1.3, pluggy-1.0.0
rootdir: C:\opt\fpdf2, configfile: setup.cfg
plugins: cov-2.11.1, icdiff-0.5, timeout-1.4.2
collected 954 items / 948 deselected / 6 selected

test\test_embed_file.py ......                                                                                                                          [100%]

====================================================================== warnings summary ====================================================================== 
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

----------- coverage: platform win32, python 3.8.3-final-0 -----------
Coverage XML written to file coverage.xml

======================================================== 6 passed, 948 deselected, 1 warning in 5.54s ========================================================

I think this has more to do with git client configuration: maybe you have core.autocrlf set to true?
You can test it with git config --get core.autocrlf

To avoid this kind of environment-dependent issues, we could adopt a .gitattributes file in this repository, that would ensure all text files are checked out with LF line endings and not CRLF

@gmischler
Copy link
Collaborator Author

I think this has more to do with git client configuration: maybe you have core.autocrlf set to true? You can test it with git config --get core.autocrlf

Ah yes, setting it to "input" and refetching the branch fixes it.

To avoid this kind of environment-dependent issues, we could adopt a .gitattributes file in this repository, that would ensure all text files are checked out with LF line endings and not CRLF

That would definitively be a good idea. 👍

@Lucas-C Lucas-C closed this as completed in da3022b Sep 5, 2022
@gmischler
Copy link
Collaborator Author

@Lucas-C , after this reconfiguration, Desktop Github now shows me the "tutorial/tuto[2-7].html" files as changed, presumably because they are stored in the repository with CRLF file endings. Might want to fix that for consistency.

@Lucas-C
Copy link
Member

Lucas-C commented Sep 5, 2022

Thanks for the notice, done!

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

No branches or pull requests

2 participants