Patches item #1675424, was opened at 2007-03-06 22:14 Message generated for change (Comment added) made by jimjjewett You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1675424&group_id=5470
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Library (Lib) Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Alan McIntyre (alanmcintyre) Assigned to: Nobody/Anonymous (nobody) Summary: Zipfile tweaks and test coverage improvement Initial Comment: 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. ---------------------------------------------------------------------- Comment By: Jim Jewett (jimjjewett) Date: 2007-03-18 23:01 Message: Logged In: YES user_id=764593 Originator: NO 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. ---------------------------------------------------------------------- Comment By: Collin Winter (collinwinter) Date: 2007-03-12 14:43 Message: Logged In: YES user_id=1344176 Originator: NO 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. ---------------------------------------------------------------------- Comment By: Alan McIntyre (alanmcintyre) Date: 2007-03-08 12:32 Message: Logged In: YES user_id=1115903 Originator: YES 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 ---------------------------------------------------------------------- Comment By: Georg Brandl (gbrandl) Date: 2007-03-07 03:18 Message: Logged In: YES user_id=849994 Originator: NO 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.) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1675424&group_id=5470 _______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches
