Merge pull request #25 from NoRedInk/use-urlretrieve

Use urlretrieve to avoid the hassle of dealing with bytes vs strs

native_package_install.py

@@ -10,10 +10,10 @@
 import tarfile
 try:
     # For Python 3.0 and later
-    from urllib.request import urlopen
+    from urllib.request import urlretrieve
 except ImportError:
     # Fall back to Python 2's urllib2
-    from urllib2 import urlopen
+    from urllib import urlretrieve
 
 import elm_package
 import exact_dependencies
@@ -97,10 +97,7 @@ def fetch_packages(vendor_dir, packages):
         url = format_tarball_url(package)
 
         print("Downloading {owner}/{project} {version}".format(**package))
-        tar_file = urlopen(url)
-        with open(tar_filename, 'w') as tar:
-            tar.write(tar_file.read())
-
+        urlretrieve(url, tar_filename)
         with tarfile.open(tar_filename) as tar:
             tar.extractall(vendor_owner_dir, members=tar.getmembers())
 
@@ -223,8 +220,12 @@ def update_source_directories(vendor_dir, elm_package_paths, native_packages):
     return repository
 
 
-def exclude_downloaded_packages(vendor_dir, packages):
-  return [x for x in packages if not os.path.isfile(format_tar_path(vendor_dir, x))]
+def exclude_existing_packages(vendor_dir, packages):
+  return [x for x in packages if not package_exists(vendor_dir, x)]
+
+
+def package_exists(vendor_dir, package):
+    return os.path.isdir(vendor_package_dir(vendor_dir, package))
 
 
 def main(native_elm_package_path, elm_package_paths, vendor_dir):
@@ -233,7 +234,7 @@ def main(native_elm_package_path, elm_package_paths, vendor_dir):
 
     raw_json = read_native_elm_package(native_elm_package_path)
     all_packages = packages_from_exact_deps(raw_json)
-    required_packages = exclude_downloaded_packages(absolute_vendor_dir, all_packages)
+    required_packages = exclude_existing_packages(absolute_vendor_dir, all_packages)
     fetch_packages(absolute_vendor_dir, required_packages)
     repository = update_source_directories(
         absolute_vendor_dir, absolute_elm_package_paths, required_packages)

tests/test_native_package_install.py

@@ -1,11 +1,12 @@
 import json
 import tarfile
 import difflib
+import shutil
 
 import native_package_install
 
 
-def test_main_does_not_download_twice(tmpdir, mocker):
+def test_main_does_not_download_twice_given_multiple_elm_packages(tmpdir, mocker):
     native_elm_package = {'elm-lang/core': '1.0.0'}
     native_elm_package_path = tmpdir.join('elm-native-package.json')
     native_elm_package_path.write(json.dumps(native_elm_package))
@@ -35,24 +36,23 @@ def test_main_does_not_download_twice(tmpdir, mocker):
 
     vendor = tmpdir.mkdir('vendor')
 
-    urlopen = mocker.patch.object(native_package_install, 'urlopen')
-    with open(str(fake_native_tarball_path)) as f:
-        urlopen.return_value = f
+    def write_tarfile(_, tar_filename):
+        shutil.copyfile(str(fake_native_tarball_path), tar_filename)
 
-        native_package_install.main(
-            str(native_elm_package_path),
-            list(map(str, (elm_package_one_path, elm_package_two_path))),
-            str(vendor))
+    mock_urlretrieve = mocker.patch.object(
+        native_package_install,
+        'urlretrieve',
+        side_effect=write_tarfile)
 
-    with open(str(fake_native_tarball_path)) as f:
-        urlopen.return_value = f
+    run_install = lambda: native_package_install.main(
+        str(native_elm_package_path),
+        list(map(str, (elm_package_one_path, elm_package_two_path))),
+        str(vendor))
 
-        native_package_install.main(
-            str(native_elm_package_path),
-            list(map(str, (elm_package_one_path, elm_package_two_path))),
-            str(vendor))
+    run_install()
+    run_install()
 
-    assert urlopen.call_count == 1
+    assert mock_urlretrieve.call_count == 1
 
 
 def test_update_source_directories_makes_minimum_changes(tmpdir):