From 4807b902443360d2db2034dc92fc93402c0a984b Mon Sep 17 00:00:00 2001 From: Clay McClure Date: Thu, 12 Jul 2012 16:51:13 -0400 Subject: [PATCH 1/9] Correct a comment --- pip/req.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pip/req.py b/pip/req.py index 6571b17bbb5..25aae3f7bab 100644 --- a/pip/req.py +++ b/pip/req.py @@ -895,7 +895,7 @@ def uninstall(self, auto_confirm=False): req.commit_uninstall() def locate_files(self): - ## FIXME: duplicates code from install_files; relevant code should + ## FIXME: duplicates code from prepare_files; relevant code should ## probably be factored out into a separate method unnamed = list(self.unnamed_requirements) reqs = list(self.requirements.values()) From 9f628623bcfb5f660b966a493cf1e2c47d30eb02 Mon Sep 17 00:00:00 2001 From: Clay McClure Date: Thu, 12 Jul 2012 17:51:17 -0400 Subject: [PATCH 2/9] Add `mock` requirement --- tests/test_pip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pip.py b/tests/test_pip.py index 10f075b98b9..41758523c35 100644 --- a/tests/test_pip.py +++ b/tests/test_pip.py @@ -663,5 +663,5 @@ def main(): if __name__ == '__main__': - sys.stderr.write("Run pip's tests using nosetests. Requires virtualenv, ScriptTest, and nose.\n") + sys.stderr.write("Run pip's tests using nosetests. Requires virtualenv, ScriptTest, mock, and nose.\n") sys.exit(1) From 6f3774de2663b6a13d7a54f624d3645297d595af Mon Sep 17 00:00:00 2001 From: Clay McClure Date: Thu, 12 Jul 2012 18:59:44 -0400 Subject: [PATCH 3/9] Ensure that $TMPDIR exists before running tests This is normally done by TestFileEnvironment if the `start_clear` parameter is True, but TestPipEnvironment and FastTestPipEnvironment explicitly set it to False. --- tests/test_pip.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_pip.py b/tests/test_pip.py index 41758523c35..114ec05d1df 100644 --- a/tests/test_pip.py +++ b/tests/test_pip.py @@ -362,6 +362,10 @@ def __init__(self, environ=None, use_distribute=None, sitecustomize=None): if sitecustomize: self._add_to_sitecustomize(sitecustomize) + # Ensure that $TMPDIR exists + if self.temp_path and not os.path.exists(self.temp_path): + os.makedirs(self.temp_path) + def _ignore_file(self, fn): if fn.endswith('__pycache__') or fn.endswith(".pyc"): result = True @@ -510,6 +514,10 @@ def __init__(self, environ=None, sitecustomize=None): assert self.root_path.exists + # Ensure that $TMPDIR exists + if self.temp_path and not os.path.exists(self.temp_path): + os.makedirs(self.temp_path) + def __del__(self): pass # shutil.rmtree(str(self.root_path), ignore_errors=True) From ac0fe94d5ced669bb1e5b5c0645b0597bf96c895 Mon Sep 17 00:00:00 2001 From: Clay McClure Date: Thu, 12 Jul 2012 19:16:16 -0400 Subject: [PATCH 4/9] Fix unicode tests to work with new temp file assertions --- tests/test_unicode.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_unicode.py b/tests/test_unicode.py index d9196e7505b..e419ac717b9 100644 --- a/tests/test_unicode.py +++ b/tests/test_unicode.py @@ -20,6 +20,6 @@ def test_install_package_that_emits_unicode(): env = reset_env() to_install = os.path.abspath(os.path.join(here, 'packages', 'BrokenEmitsUTF8')) - result = run_pip('install', to_install, expect_error=True) + result = run_pip('install', to_install, expect_error=True, expect_temp=True, quiet=True) assert '__main__.FakeError: this package designed to fail on install' in result.stdout assert 'UnicodeDecodeError' not in result.stdout From c82f6b2599485302935402476174b825e8baa2ee Mon Sep 17 00:00:00 2001 From: Clay McClure Date: Thu, 12 Jul 2012 19:00:24 -0400 Subject: [PATCH 5/9] Add a test to ensure that the temp build dir is always removed --- tests/test_cleanup.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py index 15a0508333b..548aa75eca0 100644 --- a/tests/test_cleanup.py +++ b/tests/test_cleanup.py @@ -96,6 +96,26 @@ def test_no_install_and_download_should_not_leave_build_dir(): assert not os.path.exists(env.venv_path/'/build'), "build/ dir should be deleted" +def test_temp_build_dir_removed(): + """ + It should remove the temp build dir created by pip. + + TestFileEnvironment provides a mechanism to ensure that + no temporary files are left after a test script executes, + so we can take advantage of that here by simply running + `run_pip` twice (which uses TestFileEnvironment). + """ + env = reset_env() + + # Install a requirement into a fresh environment. The temporary build + # directory should be removed. + result = run_pip('install', 'https://bitbucket.org/ianb/initools/get/tip.zip') + + # Now install the requirement again, exercising a different code path, + # and ensure that the temporary build directory is still removed. + result = run_pip('install', 'https://bitbucket.org/ianb/initools/get/tip.zip') + + def test_download_should_not_delete_existing_build_dir(): """ It should not delete build/ if existing before run the command From 723500188da5cf93e6a836d881ba9ff8646d8957 Mon Sep 17 00:00:00 2001 From: Clay McClure Date: Thu, 12 Jul 2012 19:01:27 -0400 Subject: [PATCH 6/9] Remove temp build dir even if the req is not installed (fixes #420) --- pip/req.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pip/req.py b/pip/req.py index 25aae3f7bab..cc7d4043338 100644 --- a/pip/req.py +++ b/pip/req.py @@ -1003,7 +1003,9 @@ def prepare_files(self, finder, force_root_egg_info=False, bundle=False): ##occurs when the script attempts to unpack the ##build directory + # NB: This call can result in the creation of a temporary build directory location = req_to_install.build_location(self.build_dir, not self.is_download) + ## FIXME: is the existance of the checkout good enough to use it? I don't think so. unpack = True url = None @@ -1084,7 +1086,7 @@ def prepare_files(self, finder, force_root_egg_info=False, bundle=False): self.add_requirement(subreq) if req_to_install.name not in self.requirements: self.requirements[req_to_install.name] = req_to_install - if self.is_download: + if self.is_download or req_to_install._temp_build_dir is not None: self.reqs_to_cleanup.append(req_to_install) else: self.reqs_to_cleanup.append(req_to_install) From 865af4605c46ba7554383c3281a41b16acad8b25 Mon Sep 17 00:00:00 2001 From: Clay McClure Date: Wed, 18 Jul 2012 11:10:07 -0400 Subject: [PATCH 7/9] Update AUTHORS.txt --- AUTHORS.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS.txt b/AUTHORS.txt index 8dee04e4ecc..8fa2be86334 100644 --- a/AUTHORS.txt +++ b/AUTHORS.txt @@ -6,6 +6,7 @@ Armin Ronacher Brian Rosner Carl Meyer Christian Oudard +Clay McClure Cody Soyland Daniel Holth Dave Abrahams From cc0ec4e5cb22fe0638b34cbf989eea48f4dae4b1 Mon Sep 17 00:00:00 2001 From: Marcus Smith Date: Sat, 28 Jul 2012 20:34:06 -0700 Subject: [PATCH 8/9] assert_no_temp feature tests --- tests/test_pip.py | 4 ++-- tests/test_test.py | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/test_pip.py b/tests/test_pip.py index 114ec05d1df..dba386fef4a 100644 --- a/tests/test_pip.py +++ b/tests/test_pip.py @@ -362,7 +362,7 @@ def __init__(self, environ=None, use_distribute=None, sitecustomize=None): if sitecustomize: self._add_to_sitecustomize(sitecustomize) - # Ensure that $TMPDIR exists + # Ensure that $TMPDIR exists (because we use start_clear=False, it's not created for us) if self.temp_path and not os.path.exists(self.temp_path): os.makedirs(self.temp_path) @@ -514,7 +514,7 @@ def __init__(self, environ=None, sitecustomize=None): assert self.root_path.exists - # Ensure that $TMPDIR exists + # Ensure that $TMPDIR exists (because we use start_clear=False, it's not created for us) if self.temp_path and not os.path.exists(self.temp_path): os.makedirs(self.temp_path) diff --git a/tests/test_test.py b/tests/test_test.py index a5293957526..498652d959f 100644 --- a/tests/test_test.py +++ b/tests/test_test.py @@ -66,3 +66,24 @@ def test_sitecustomize_not_growing_in_fast_environment(): size2 = os.stat(sc2).st_size assert size1==size2, "size before, %d != size after, %d" %(size1, size2) + +def test_tmp_dir_exists_in_env(): + """ + Test that $TMPDIR == env.temp_path and path exists, and env.assert_no_temp() passes + """ + #need these tests to ensure the assert_no_temp feature of scripttest is working + env = reset_env(use_distribute=True) + env.assert_no_temp() #this fails if env.tmp_path doesn't exist + assert env.environ['TMPDIR'] == env.temp_path + assert isdir(env.temp_path) + + +def test_tmp_dir_exists_in_fast_env(): + """ + Test that $TMPDIR == env.temp_path and path exists and env.assert_no_temp() passes (in fast env) + """ + #need these tests to ensure the assert_no_temp feature of scripttest is working + env = reset_env() + env.assert_no_temp() #this fails if env.tmp_path doesn't exist + assert env.environ['TMPDIR'] == env.temp_path + assert isdir(env.temp_path) From bb5b92e75438dc1b2c7909bf8f088bd7280ac64c Mon Sep 17 00:00:00 2001 From: Marcus Smith Date: Sat, 28 Jul 2012 21:09:07 -0700 Subject: [PATCH 9/9] issue #420 test enhancement --- tests/test_cleanup.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py index 548aa75eca0..c57db91ee3d 100644 --- a/tests/test_cleanup.py +++ b/tests/test_cleanup.py @@ -17,6 +17,7 @@ def test_cleanup_after_install_from_pypi(): src = env.scratch_path/"src" assert not exists(build), "build/ dir still exists: %s" % build assert not exists(src), "unexpected src/ dir exists: %s" % src + env.assert_no_temp() def test_cleanup_after_install_editable_from_hg(): @@ -34,6 +35,7 @@ def test_cleanup_after_install_editable_from_hg(): src = env.venv_path/'src' assert not exists(build), "build/ dir still exists: %s" % build assert exists(src), "expected src/ dir doesn't exist: %s" % src + env.assert_no_temp() def test_cleanup_after_install_from_local_directory(): @@ -48,6 +50,7 @@ def test_cleanup_after_install_from_local_directory(): src = env.venv_path/'src' assert not exists(build), "unexpected build/ dir exists: %s" % build assert not exists(src), "unexpected src/ dir exist: %s" % src + env.assert_no_temp() def test_cleanup_after_create_bundle(): @@ -79,6 +82,7 @@ def test_cleanup_after_create_bundle(): src_bundle = env.scratch_path/"src-bundle" assert not exists(build_bundle), "build-bundle/ dir still exists: %s" % build_bundle assert not exists(src_bundle), "src-bundle/ dir still exists: %s" % src_bundle + env.assert_no_temp() # Make sure previously created src/ from editable still exists assert exists(src), "expected src dir doesn't exist: %s" % src @@ -96,24 +100,23 @@ def test_no_install_and_download_should_not_leave_build_dir(): assert not os.path.exists(env.venv_path/'/build'), "build/ dir should be deleted" -def test_temp_build_dir_removed(): +def test_cleanup_req_satisifed_no_name(): """ - It should remove the temp build dir created by pip. - - TestFileEnvironment provides a mechanism to ensure that - no temporary files are left after a test script executes, - so we can take advantage of that here by simply running - `run_pip` twice (which uses TestFileEnvironment). + Test cleanup when req is already satisfied, and req has no 'name' """ - env = reset_env() - - # Install a requirement into a fresh environment. The temporary build - # directory should be removed. - result = run_pip('install', 'https://bitbucket.org/ianb/initools/get/tip.zip') + #this test confirms Issue #420 is fixed + #reqs with no 'name' that were already satisfied were leaving behind tmp build dirs + #2 examples of reqs that would do this + # 1) https://bitbucket.org/ianb/initools/get/tip.zip + # 2) parent-0.1.tar.gz - # Now install the requirement again, exercising a different code path, - # and ensure that the temporary build directory is still removed. - result = run_pip('install', 'https://bitbucket.org/ianb/initools/get/tip.zip') + dist = abspath(join(here, 'packages', 'parent-0.1.tar.gz')) + env = reset_env() + result = run_pip('install', dist) + result = run_pip('install', dist) + build = env.venv_path/'build' + assert not exists(build), "unexpected build/ dir exists: %s" % build + env.assert_no_temp() def test_download_should_not_delete_existing_build_dir():