This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Zipfile tweaks and test coverage improvement
Type: Stage:
Components: Library (Lib) Versions:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: Nosy List: alanmcintyre, collinwinter, georg.brandl, jimjjewett
Priority: normal Keywords: patch

Created on 2007-03-07 03:14 by alanmcintyre, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
zipfile_coverage.diff alanmcintyre, 2007-03-07 03:14 diff against rev 54190
zipfile_coverage2.diff alanmcintyre, 2007-03-08 17:32 updated (diff against ref 54219)
Messages (7)
msg52075 - (view) Author: Alan McIntyre (alanmcintyre) * (Python committer) Date: 2007-03-07 03:14
This patch adds 12 tests for zipfile behavior that didn't appear to be covered by existing tests:

  - appending to an existing zipfile
  - appending to an existing file that is not a zipfile
  - check that calling ZipFile.write without arcname specified produces the expected result
  - check that files within a Zip archive can have different compression options
  - check that trying to call write() on a readonly ZipFile object raises a RuntimeError
  - check that PyZipFile.writepy won't accept a file that doesn't have a .py extension
  - check that bad modes passed to ZipFile constructor are caught
  - check that bad modes passed to ZipFile.open are caught
  - check that calling read(0) on a ZipExtFile object returns an empty string and doesn't advance file pointer
  - check that attempting to call open() for an item that doesn't exist in the archive raises a RuntimeError
  - check that bad compression methods passed to ZipFile.open are caught
  - check that a filename containing a null byte is properly terminated

These other miscellaneous changes were made to test_zipfile.py:
  - FIXEDTEST_SIZE was changed to 1000 (I mistakenly left it at 10 instead of the original test size of 1000 when I last posted patch 1121142)
  - An existing test that checks for a RuntimeError when calling testzip on a closed ZipFile also now checks for the same exception on calls to read, open, write, and writestr.
  - In TestsWithSourceFile, the line_gen attribute was changed from a generator to a list because some of the tests iterate over lines in the test data.

The following changes were made to zipfile.ZipFile:
- A check was added to ZipFile constructor to check for bogus modes (and a couple of statements before the previously existing argument validation were moved below said validation)
- ZipFile.getinfo now raises a RuntimeError when attempting to retrieve information for a file not in the archive (this seems more helpful than getting a cryptic KeyError)
- checks were added to ZipFile.write and writestr to raise a RuntimeError when attempting to write to a closed ZipFile (note that this probably makes other checks further down in the code for these methods redundant)

There are still some portions of zipfile.py that aren't covered, but if somebody commits this patch before I get around to writing tests for them I won't complain about having to submit a separate patch for them. ;-)

If anybody has any thoughts on whether or not things like "calling ZipFile.getinfo for an item not in the archive raises a RuntimeError" should be in the docs, please let me know.  I can add such documentation changes if necessary.
msg52076 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2007-03-07 08:18
I'd continue raising KeyError in the case the info wasn't found,
but set a different message so that backwards compatibility doesn't break.
In any case, it would be good to document it.

Isn't raising IOError customary for writing to closed files? (If zipfile
never raises IOError elsewhere, this is probably not applicable.)
msg52077 - (view) Author: Alan McIntyre (alanmcintyre) * (Python committer) Date: 2007-03-08 17:32
I've modified ZipFile.getinfo to raise a KeyError instead of a RuntimeError.  It would appear that RuntimeError is what stuff in this module raises for IO problems, so I'm assuming this means we're stuck with it (even though I agree that IOError would seem more appropriate for trying to write to closed files). 

I documented some of the exceptions raised by various methods. However, I focused mainly on the items listed in my original post, so there's probably code that raise exceptions that still aren't properly documented.

I also noticed that the existing docs list "error" as the exception that is raised for bad zip files.  Since no such exception is raised, I included a change to the docs so that it says BadZipfile instead. 

I included a change to the NEWS file as well, since it was mentioned in py-dev discussion that such updates can be helpful. :-)
File Added: zipfile_coverage2.diff
msg52078 - (view) Author: Collin Winter (collinwinter) * (Python committer) Date: 2007-03-12 18:43
I'm definitely in favor of the docs and test changes, and the extra error checking seems like a good idea. Both test_zipfile and test_zipfile64 pass for me with the patch applied.
msg52079 - (view) Author: Jim Jewett (jimjjewett) Date: 2007-03-19 03:01
It is possible to define a new Exception class which inherits from both RuntimeError and IOError, and raise that.  Then document IOError, with a footnote mentioning that in 2.x it is also a RuntimeError for backwards compatibility.  

clientcookie does something similar; LoadError is derived from IOError only for backwards compatibility.
msg52080 - (view) Author: Jim Jewett (jimjjewett) Date: 2007-03-19 03:02
sorry; I meant "cookielib"; clientcookie is the name of the independently distributed package.  (Though last I checked, John Lee was using roughly the same code in both.)
msg52081 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2007-07-12 09:59
Committed as rev. 56308.
History
Date User Action Args
2022-04-11 14:56:23adminsetgithub: 44671
2007-03-07 03:14:33alanmcintyrecreate