claird / PyPDF4

A utility to read and write PDFs with Python
obsolete-https://pythonhosted.org/PyPDF2/
Other
328 stars 61 forks source link

Adding attachFiles function and unit tests #29

Closed michd89 closed 4 years ago

michd89 commented 5 years ago
acsor commented 5 years ago

Hey @michd89, some of the test data is missing from your (two?) tests. Nothing serious, you should just commit them as well (track them into the staging area → commit → push again).

Here are the error backlogs under my setup:

testAddAttachment()

Error
Traceback (most recent call last):
  File "/usr/lib/python3.5/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/lib/python3.5/unittest/case.py", line 601, in run
    testMethod()
  File "/home/acsor/software/PyPDF4/tests/test_pdf.py", line 443, in testAddAttachment
    'jpeg.pdf')) as writer:
  File "/home/acsor/software/PyPDF4/pypdf/pdf.py", line 82, in __init__
    self._stream = open(stream, "wb")
FileNotFoundError: [Errno 2] No such file or directory: '/home/acsor/software/PyPDF4/tests/fixture_data/testAddAttachment/jpeg.pdf'

testAttachFiles()

Exception ignored in: <object repr() failed>
Traceback (most recent call last):

Error
Traceback (most recent call last):
  File "/usr/lib/python3.5/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/lib/python3.5/unittest/case.py", line 601, in run
    testMethod()
  File "/home/acsor/software/PyPDF4/tests/test_pdf.py", line 476, in testAttachFiles
    'jpeg.pdf')) as writer:
  File "/home/acsor/software/PyPDF4/pypdf/pdf.py", line 82, in __init__
    self._stream = open(stream, "wb")
FileNotFoundError: [Errno 2] No such file or directory: '/home/acsor/software/PyPDF4/tests/fixture_data/testAddAttachment/jpeg.pdf'
michd89 commented 5 years ago

@newnone Indeed, I left some paths within the tests, which existed only in my local repository. Now I replaced them by temporary files.

acsor commented 5 years ago

All seems to be fine now. By the way, I suppose you have guessed the structure of the test suite. If need to be, you can place test files within dedicated directories (e.g. tests/fixture_data/testAddAttachment/).

Until I find time to carefully analyze how these changes interfere with the preexisting code, I leave it to @claird whether or not this PR can be merged.

michd89 commented 5 years ago

Yes, in the beginning I thought about using dedicated directories (I kept them in my early tests; that's why they failed before the fix). But actually the files which are already provided in tests/fixture_data/ are suitable.

Between the addAttachment and attachFiles there are some redundancies because my priority was to add the new functionality in the latter one. I'm sure that @claird wants to resolve some redundancies if possible.

Edit: Typo