diff --git a/gsutil.py b/gsutil.py index eb339bc41..e8e9ca3cd 100755 --- a/gsutil.py +++ b/gsutil.py @@ -72,12 +72,6 @@ def download_gsutil(version, target_dir): return target_filename -def check_gsutil(gsutil_bin): - """Run gsutil version and make sure it runs.""" - return subprocess.call( - [sys.executable, gsutil_bin, 'version'], - stdout=subprocess.PIPE, stderr=subprocess.STDOUT) == 0 - @contextlib.contextmanager def temporary_directory(base): tmpdir = tempfile.mkdtemp(prefix='gsutil_py', dir=base) @@ -90,7 +84,10 @@ def temporary_directory(base): def ensure_gsutil(version, target, clean): bin_dir = os.path.join(target, 'gsutil_%s' % version) gsutil_bin = os.path.join(bin_dir, 'gsutil', 'gsutil') - if not clean and os.path.isfile(gsutil_bin) and check_gsutil(gsutil_bin): + gsutil_flag = os.path.join(bin_dir, 'gsutil', 'install.flag') + # We assume that if gsutil_flag exists, then we have a good version + # of the gsutil package. + if not clean and os.path.isfile(gsutil_flag): # Everything is awesome! we're all done here. return gsutil_bin @@ -116,10 +113,13 @@ def ensure_gsutil(version, target, clean): except (OSError, IOError): # Something else did this in parallel. pass + # Final check that the gsutil bin exists. This should never fail. + if not os.path.isfile(gsutil_bin): + raise InvalidGsutilError() + # Drop a flag file. + with open(gsutil_flag, 'w') as f: + f.write('This flag file is dropped by gsutil.py') - # Final check that the gsutil bin is okay. This should never fail. - if not check_gsutil(gsutil_bin): - raise InvalidGsutilError() return gsutil_bin diff --git a/tests/download_from_google_storage_unittests.py b/tests/download_from_google_storage_unittest.py similarity index 95% rename from tests/download_from_google_storage_unittests.py rename to tests/download_from_google_storage_unittest.py index d2ed8a8e1..1c285f491 100755 --- a/tests/download_from_google_storage_unittests.py +++ b/tests/download_from_google_storage_unittest.py @@ -66,6 +66,9 @@ class GsutilMock(object): else: return (0, '', '') + def check_call_with_retries(self, *args): + return self.check_call(*args) + class ChangedWorkingDirectory(object): def __init__(self, working_directory): @@ -135,6 +138,7 @@ class GstoolsUnitTests(unittest.TestCase): tar_dir)) def test_gsutil(self): + # This will download a real gsutil package from Google Storage. gsutil = download_from_google_storage.Gsutil(GSUTIL_DEFAULT_PATH, None) self.assertEqual(gsutil.path, GSUTIL_DEFAULT_PATH) code, _, err = gsutil.check_call() @@ -190,7 +194,7 @@ class DownloadTests(unittest.TestCase): self.parser = optparse.OptionParser() self.queue = Queue.Queue() self.ret_codes = Queue.Queue() - self.lorem_ipsum = os.path.join(self.base_path, 'lorem_ipsum.txt') + self.lorem_ipsum = os.path.join(TEST_DIR, 'gstools', 'lorem_ipsum.txt') self.lorem_ipsum_sha1 = '7871c8e24da15bad8b0be2c36edc9dc77e37727f' self.maxDiff = None @@ -222,9 +226,12 @@ class DownloadTests(unittest.TestCase): self.assertEqual(queue_size, 3) def test_download_worker_single_file(self): - sha1_hash = '7871c8e24da15bad8b0be2c36edc9dc77e37727f' + sha1_hash = self.lorem_ipsum_sha1 input_filename = '%s/%s' % (self.base_url, sha1_hash) output_filename = os.path.join(self.base_path, 'uploaded_lorem_ipsum.txt') + self.gsutil.add_expected(0, '', '') # ls + self.gsutil.add_expected(0, '', '', lambda: shutil.copyfile( + self.lorem_ipsum, output_filename)) # cp self.queue.put((sha1_hash, output_filename)) self.queue.put((None, None)) stdout_queue = Queue.Queue() @@ -257,10 +264,8 @@ class DownloadTests(unittest.TestCase): download_from_google_storage._downloader_worker_thread( 0, self.queue, False, self.base_url, self.gsutil, stdout_queue, self.ret_codes, True, False) - expected_output = [ - '0> File %s exists and SHA1 matches. Skipping.' % output_filename - ] - self.assertEqual(list(stdout_queue.queue), expected_output) + # dfgs does not output anything in the no-op case. + self.assertEqual(list(stdout_queue.queue), []) self.assertEqual(self.gsutil.history, []) def test_download_extract_archive(self): @@ -354,11 +359,6 @@ class DownloadTests(unittest.TestCase): ('check_call', ('cp', input_filename, output_filename)) ] - if sys.platform != 'win32': - expected_calls.append( - ('check_call', - ('stat', - 'gs://sometesturl/7871c8e24da15bad8b0be2c36edc9dc77e37727f'))) self.assertEqual(self.gsutil.history, expected_calls) self.assertEqual(code, 101) @@ -392,6 +392,9 @@ class DownloadTests(unittest.TestCase): sha1_hash = '7871c8e24da15bad8b0be2c36edc9dc77e37727f' input_filename = '%s/%s' % (self.base_url, sha1_hash) output_filename = os.path.join(self.base_path, 'uploaded_lorem_ipsum.txt') + self.gsutil.add_expected(0, '', '') # ls + self.gsutil.add_expected(0, '', '', lambda: shutil.copyfile( + self.lorem_ipsum, output_filename)) # cp code = download_from_google_storage.download_from_google_storage( input_filename=self.base_path, base_url=self.base_url, diff --git a/tests/gsutil_test.py b/tests/gsutil_test.py index d34eebfa5..d0fb8b029 100755 --- a/tests/gsutil_test.py +++ b/tests/gsutil_test.py @@ -122,14 +122,9 @@ class GsutilUnitTests(unittest.TestCase): version = '4.2' gsutil_dir = os.path.join(self.tempdir, 'gsutil_%s' % version, 'gsutil') gsutil_bin = os.path.join(gsutil_dir, 'gsutil') + gsutil_flag = os.path.join(gsutil_dir, 'install.flag') os.makedirs(gsutil_dir) - self.fake.add_expectation( - [sys.executable, gsutil_bin, 'version'], stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, _returns=1) - - with open(gsutil_bin, 'w') as f: - f.write('Foobar') zip_filename = 'gsutil_%s.zip' % version url = '%s%s' % (gsutil.GSUTIL_URL, zip_filename) _, tempzip = tempfile.mkstemp() @@ -138,32 +133,26 @@ class GsutilUnitTests(unittest.TestCase): zf.writestr('gsutil/gsutil', fake_gsutil) with open(tempzip, 'rb') as f: self.fake.add_expectation(url, _returns=Buffer(f.read())) - self.fake.add_expectation( - [sys.executable, gsutil_bin, 'version'], stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, _returns=1) - - # This should delete the old bin and rewrite it with 'Fake gsutil' - self.assertRaises( - gsutil.InvalidGsutilError, gsutil.ensure_gsutil, version, self.tempdir, - False) + + # This should write the gsutil_bin with 'Fake gsutil' + gsutil.ensure_gsutil(version, self.tempdir, False) self.assertTrue(os.path.exists(gsutil_bin)) with open(gsutil_bin, 'r') as f: self.assertEquals(f.read(), fake_gsutil) + self.assertTrue(os.path.exists(gsutil_flag)) self.assertEquals(self.fake.expectations, []) def test_ensure_gsutil_short(self): version = '4.2' gsutil_dir = os.path.join(self.tempdir, 'gsutil_%s' % version, 'gsutil') gsutil_bin = os.path.join(gsutil_dir, 'gsutil') + gsutil_flag = os.path.join(gsutil_dir, 'install.flag') os.makedirs(gsutil_dir) - # Mock out call(). - self.fake.add_expectation( - [sys.executable, gsutil_bin, 'version'], - stdout=subprocess.PIPE, stderr=subprocess.STDOUT, _returns=0) - with open(gsutil_bin, 'w') as f: f.write('Foobar') + with open(gsutil_flag, 'w') as f: + f.write('Barbaz') self.assertEquals( gsutil.ensure_gsutil(version, self.tempdir, False), gsutil_bin) diff --git a/tests/upload_to_google_storage_unittests.py b/tests/upload_to_google_storage_unittest.py similarity index 98% rename from tests/upload_to_google_storage_unittests.py rename to tests/upload_to_google_storage_unittest.py index 3f37823e4..ed8eca1eb 100755 --- a/tests/upload_to_google_storage_unittests.py +++ b/tests/upload_to_google_storage_unittest.py @@ -19,8 +19,8 @@ import unittest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import upload_to_google_storage -from download_from_google_storage_unittests import GsutilMock -from download_from_google_storage_unittests import ChangedWorkingDirectory +from download_from_google_storage_unittest import GsutilMock +from download_from_google_storage_unittest import ChangedWorkingDirectory # ../third_party/gsutil/gsutil GSUTIL_DEFAULT_PATH = os.path.join(