Moved verbosity to the helper function, added unit tests, prints packages, size, and signatures (#652)

Author: VikramJayanthi17

tests/test_upload.py

@@ -11,6 +11,8 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+import os
+
 import pretend
 import pytest
 import requests
@@ -52,6 +54,50 @@ def upload_settings(make_settings, stub_repository):
     return upload_settings
 
 
+def test_make_package_pre_signed_dist(upload_settings, capsys):
+    """Create a PackageFile and print path, size, and user-provided signature."""
+    filename = helpers.WHEEL_FIXTURE
+    expected_size = "15.4 KB"
+    signed_filename = helpers.WHEEL_FIXTURE + ".asc"
+    signatures = {os.path.basename(signed_filename): signed_filename}
+
+    upload_settings.sign = True
+    upload_settings.verbose = True
+
+    package = upload._make_package(filename, signatures, upload_settings)
+
+    assert package.filename == filename
+    assert package.gpg_signature is not None
+
+    captured = capsys.readouterr()
+    assert captured.out.count(f"{filename} ({expected_size})") == 1
+    assert captured.out.count(f"Signed with {signed_filename}") == 1
+
+
+def test_make_package_unsigned_dist(upload_settings, monkeypatch, capsys):
+    """Create a PackageFile and print path, size, and Twine-generated signature."""
+    filename = helpers.NEW_WHEEL_FIXTURE
+    expected_size = "21.9 KB"
+    signatures = {}
+
+    upload_settings.sign = True
+    upload_settings.verbose = True
+
+    def stub_sign(package, *_):
+        package.gpg_signature = (package.signed_basefilename, b"signature")
+
+    monkeypatch.setattr(package_file.PackageFile, "sign", stub_sign)
+
+    package = upload._make_package(filename, signatures, upload_settings)
+
+    assert package.filename == filename
+    assert package.gpg_signature is not None
+
+    captured = capsys.readouterr()
+    assert captured.out.count(f"{filename} ({expected_size})") == 1
+    assert captured.out.count(f"Signed with {package.signed_filename}") == 1
+
+
 def test_successs_prints_release_urls(upload_settings, stub_repository, capsys):
     """Print PyPI release URLS for each uploaded package."""
     stub_repository.release_urls = lambda packages: {RELEASE_URL, NEW_RELEASE_URL}
@@ -72,6 +118,26 @@ def test_successs_prints_release_urls(upload_settings, stub_repository, capsys):
     assert captured.out.count(NEW_RELEASE_URL) == 1
 
 
+def test_print_packages_if_verbose(upload_settings, capsys):
+    """Print the path and file size of each distribution attempting to be uploaded."""
+    dists_to_upload = {
+        helpers.WHEEL_FIXTURE: "15.4 KB",
+        helpers.SDIST_FIXTURE: "20.8 KB",
+        helpers.NEW_SDIST_FIXTURE: "26.1 KB",
+        helpers.NEW_WHEEL_FIXTURE: "21.9 KB",
+    }
+
+    upload_settings.verbose = True
+
+    result = upload.upload(upload_settings, dists_to_upload.keys())
+    assert result is None
+
+    captured = capsys.readouterr()
+
+    for filename, size in dists_to_upload.items():
+        assert captured.out.count(f"{filename} ({size})") == 1
+
+
 def test_success_with_pre_signed_distribution(upload_settings, stub_repository):
     """Add GPG signature provided by user to uploaded package."""
     # Upload a pre-signed distribution

tests/test_utils.py

@@ -294,3 +294,16 @@ def test_check_status_code_for_missing_status_code(capsys, repo_url):
 
     captured = capsys.readouterr()
     assert captured.out == "NOTE: Try --verbose to see response content.\n"
+
+
+@pytest.mark.parametrize(
+    ("size_in_bytes, formatted_size"),
+    [(3704, "3.6 KB"), (1153433, "1.1 MB"), (21412841, "20.4 MB")],
+)
+def test_get_file_size(size_in_bytes, formatted_size, monkeypatch):
+    """Get the size of file as a string with units."""
+    monkeypatch.setattr(os.path, "getsize", lambda _: size_in_bytes)
+
+    file_size = utils.get_file_size(size_in_bytes)
+
+    assert file_size == formatted_size

twine/commands/upload.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 import argparse
 import os.path
+from typing import Dict
 from typing import List
 from typing import cast
 
@@ -50,24 +51,45 @@ def skip_upload(
     )
 
 
+def _make_package(
+    filename: str, signatures: Dict[str, str], upload_settings: settings.Settings
+) -> package_file.PackageFile:
+    """Create and sign a package, based off of filename, signatures and settings."""
+    package = package_file.PackageFile.from_filename(filename, upload_settings.comment)
+
+    signed_name = package.signed_basefilename
+    if signed_name in signatures:
+        package.add_gpg_signature(signatures[signed_name], signed_name)
+    elif upload_settings.sign:
+        package.sign(upload_settings.sign_with, upload_settings.identity)
+
+    if upload_settings.verbose:
+        file_size = utils.get_file_size(package.filename)
+        print(f"  {package.filename} ({file_size})")
+        if package.gpg_signature:
+            print(f"  Signed with {package.signed_filename}")
+
+    return package
+
+
 def upload(upload_settings: settings.Settings, dists: List[str]) -> None:
     dists = commands._find_dists(dists)
-
     # Determine if the user has passed in pre-signed distributions
     signatures = {os.path.basename(d): d for d in dists if d.endswith(".asc")}
     uploads = [i for i in dists if not i.endswith(".asc")]
+
     upload_settings.check_repository_url()
     repository_url = cast(str, upload_settings.repository_config["repository"])
-
     print(f"Uploading distributions to {repository_url}")
 
+    packages_to_upload = [
+        _make_package(filename, signatures, upload_settings) for filename in uploads
+    ]
+
     repository = upload_settings.create_repository()
     uploaded_packages = []
 
-    for filename in uploads:
-        package = package_file.PackageFile.from_filename(
-            filename, upload_settings.comment
-        )
+    for package in packages_to_upload:
         skip_message = "  Skipping {} because it appears to already exist".format(
             package.basefilename
         )
@@ -79,12 +101,6 @@ def upload(upload_settings: settings.Settings, dists: List[str]) -> None:
             print(skip_message)
             continue
 
-        signed_name = package.signed_basefilename
-        if signed_name in signatures:
-            package.add_gpg_signature(signatures[signed_name], signed_name)
-        elif upload_settings.sign:
-            package.sign(upload_settings.sign_with, upload_settings.identity)
-
         resp = repository.upload(package)
 
         # Bug 92. If we get a redirect we should abort because something seems

twine/utils.py

@@ -159,6 +159,18 @@ def normalize_repository_url(url: str) -> str:
     return urlunparse(parsed)
 
 
+def get_file_size(filename: str) -> str:
+    """Return the size of a file in KB, or MB if >= 1024 KB."""
+    file_size = os.path.getsize(filename) / 1024
+    size_unit = "KB"
+
+    if file_size > 1024:
+        file_size = file_size / 1024
+        size_unit = "MB"
+
+    return f"{file_size:.1f} {size_unit}"
+
+
 def check_status_code(response: requests.Response, verbose: bool) -> None:
     """Generate a helpful message based on the response from the repository.