Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(95176)

#17633: zipimport's handling of namespace packages is incorrect

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 4 months ago by pconnell
Modified:
3 years, 7 months ago
Reviewers:
storchaka, brett
CC:
brett.cannon, romberg, theller, gregory.p.smith, Nick Coghlan, eric.smith, Arfrever, devnull_psf.upfronthosting.co.za, eric.snow, pconnell, isoschiz, superluser
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Total comments: 2

Patch Set 4 #

Patch Set 5 #

Total comments: 15

Patch Set 6 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/test/test_zipimport.py View 1 2 3 4 5 8 chunks +216 lines, -32 lines 0 comments Download
Modules/zipimport.c View 1 2 3 4 5 4 chunks +32 lines, -16 lines 0 comments Download

Messages

Total messages: 4
storchaka_gmail.com
http://bugs.python.org/review/17633/diff/7925/Modules/zipimport.c File Modules/zipimport.c (right): http://bugs.python.org/review/17633/diff/7925/Modules/zipimport.c#newcode366 Modules/zipimport.c:366: if (is_dir && result != -1) { else if ...
6 years, 4 months ago #1
pconnell
http://bugs.python.org/review/17633/diff/7925/Modules/zipimport.c File Modules/zipimport.c (right): http://bugs.python.org/review/17633/diff/7925/Modules/zipimport.c#newcode366 Modules/zipimport.c:366: if (is_dir && result != -1) { On 2013/04/18 ...
6 years, 4 months ago #2
brett.cannon
Approach LGTM, so mostly comments on style. http://bugs.python.org/review/17633/diff/16283/Lib/test/test_zipimport.py File Lib/test/test_zipimport.py (right): http://bugs.python.org/review/17633/diff/16283/Lib/test/test_zipimport.py#newcode87 Lib/test/test_zipimport.py:87: def makeTree(self, ...
3 years, 7 months ago #3
storchaka_gmail.com
3 years, 7 months ago #4
http://bugs.python.org/review/17633/diff/16283/Lib/test/test_zipimport.py
File Lib/test/test_zipimport.py (right):

http://bugs.python.org/review/17633/diff/16283/Lib/test/test_zipimport.py#new...
Lib/test/test_zipimport.py:87: def makeTree(self, files, dirName=TEMP_DIR):
On 2016/01/09 00:57:36, brett.cannon wrote:
> I don't see any guarantee that the files will be cleaned  up after the test.
> Should probably use unittest.TestCase.addCleanup() to make sure everything
that
> is created is also deleted.

And it is better to use support.rmtree instead of shutil.rmtree.

http://bugs.python.org/review/17633/diff/16283/Lib/test/test_zipimport.py#new...
Lib/test/test_zipimport.py:103: z = ZipFile(zipName, "w")
Use "with".

http://bugs.python.org/review/17633/diff/16283/Lib/test/test_zipimport.py#new...
Lib/test/test_zipimport.py:113: with open(TEMP_ZIP, "rb") as f:
TEMP_ZIP, not zipName?
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+