From 0f06e295b51ead81bc80bda0a93a863a32143ac8 Mon Sep 17 00:00:00 2001 From: Daniele Nicolodi Date: Thu, 19 Dec 2024 15:59:37 +0100 Subject: [PATCH 1/4] Add helpers to create test sdists and wheels --- tests/helpers.py | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/helpers.py b/tests/helpers.py index b1ea8068..4d862f50 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -44,10 +44,54 @@ def build_archive(path, name, archive_format, files): archive.addfile(member, io.BytesIO(data)) return filepath - if archive_format == "zip": + if archive_format == "zip" or archive_format == "whl": with zipfile.ZipFile(filepath, mode="w") as archive: for mname, content in files.items(): archive.writestr(mname, textwrap.dedent(content)) return filepath raise ValueError(format) + + +def build_sdist(path, name, version, files): + content = { + f"{name}-{version}/README": "README", + f"{name}-{version}/PKG-INFO": f"""\ + Metadata-Version: 1.1 + Name: {name} + Version: {version} + """, + } + + # Remap file paths to be under archive root folder. + root = f"{name}-{version}" + for key, value in files.items(): + content[f"{root}/{key}"] = value + + return build_archive(path, f"{name}-{version}", "tar.gz", content) + + +def build_wheel(path, name, version, files): + # This does not generate valid a wheel file: the archive lacks the + # .dist-info/RECORD member with the content listing. However, the + # generated files pass validation done by twine and are sufficient + # for testing purposes. + + content = { + f"{name}-{version}.dist-info/WHEEL": """\ + Wheel-Version: 1.0 + Generator: test + Root-Is-Purelib: true + Tag: py3-none-any + """, + f"{name}-{version}.dist-info/METADATA": f"""\ + Metadata-Version: 1.1 + Name: {name} + Version: {version} + """, + } + + # Add wheel content. + content.update(files) + + return build_archive(path, f"{name}-{version}-py3-none-any", "whl", content) From 11861996044cb0d782f5497bec9348a9d32e945e Mon Sep 17 00:00:00 2001 From: Daniele Nicolodi Date: Wed, 18 Dec 2024 13:55:34 +0100 Subject: [PATCH 2/4] Add pytest fixtures to get the path to a test sdist and test wheel To avoid creating these over and over again, create the sdist and wheel in a temporary direcory with lifetime related to the test session scope. --- tests/conftest.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 266cfee3..c5c2777d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,7 @@ import getpass import logging.config +import pathlib +import tempfile import textwrap import pytest @@ -8,6 +10,9 @@ from twine import settings from twine import utils +from .helpers import build_sdist +from .helpers import build_wheel + @pytest.fixture(autouse=True) def configure_output(): @@ -83,3 +88,31 @@ def _settings(config=default_config, **settings_kwargs): @pytest.fixture def entered_password(monkeypatch): monkeypatch.setattr(getpass, "getpass", lambda prompt: "entered pw") + + +@pytest.fixture(scope="session") +def tmp_path_session(tmp_path_factory): + return pathlib.Path( + tempfile.mkdtemp( + prefix="twine-test-", + dir=tmp_path_factory.mktemp("test"), + ) + ) + + +@pytest.fixture +def test_wheel(tmp_path_session): + return build_wheel(tmp_path_session, "test", "1.2.3", {}) + + +@pytest.fixture +def test_wheel_signature(test_wheel): + signature = test_wheel.with_suffix(".whl.asc") + signature.write_text("-----BEGIN PGP SIGNATURE-----") + yield signature + signature.unlink() + + +@pytest.fixture +def test_sdist(tmp_path_session): + return build_sdist(tmp_path_session, "test", "1.2.3", {}) From 1591d6bbc9578f775a10ccf835d6d7a7d32add1a Mon Sep 17 00:00:00 2001 From: Daniele Nicolodi Date: Wed, 18 Dec 2024 13:56:37 +0100 Subject: [PATCH 3/4] Move test_register away from on-disc test fixtures The tests in this module do not care about the content of the wheel. --- tests/test_register.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/tests/test_register.py b/tests/test_register.py index 2235d415..9591ee9c 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -5,8 +5,6 @@ from twine import exceptions from twine.commands import register -from . import helpers - @pytest.fixture() def register_settings(make_settings): @@ -21,7 +19,7 @@ def register_settings(make_settings): ) -def test_successful_register(register_settings): +def test_successful_register(register_settings, test_wheel): """Return a successful result for a valid repository url and package.""" stub_response = pretend.stub( is_redirect=False, @@ -36,12 +34,12 @@ def test_successful_register(register_settings): register_settings.create_repository = lambda: stub_repository - result = register.register(register_settings, helpers.WHEEL_FIXTURE) + result = register.register(register_settings, str(test_wheel)) assert result is None -def test_exception_for_redirect(register_settings): +def test_exception_for_redirect(register_settings, test_wheel): """Raise an exception when repository URL results in a redirect.""" repository_url = register_settings.repository_config["repository"] redirect_url = "https://malicious.website.org/danger/" @@ -62,10 +60,10 @@ def test_exception_for_redirect(register_settings): exceptions.RedirectDetected, match=rf"{repository_url}.+{redirect_url}.+\nIf you trust these URLs", ): - register.register(register_settings, helpers.WHEEL_FIXTURE) + register.register(register_settings, str(test_wheel)) -def test_non_existent_package(register_settings): +def test_non_existent_package(register_settings, test_wheel): """Raise an exception when package file doesn't exist.""" stub_repository = pretend.stub() @@ -80,7 +78,7 @@ def test_non_existent_package(register_settings): @pytest.mark.parametrize("repo", ["pypi", "testpypi"]) -def test_values_from_env_pypi(monkeypatch, repo): +def test_values_from_env_pypi(monkeypatch, repo, test_wheel): """Use env vars for settings when run from command line.""" def none_register(*args, **settings_kwargs): @@ -96,14 +94,14 @@ def none_register(*args, **settings_kwargs): } for key, value in testenv.items(): monkeypatch.setenv(key, value) - cli.dispatch(["register", helpers.WHEEL_FIXTURE]) + cli.dispatch(["register", str(test_wheel)]) register_settings = replaced_register.calls[0].args[0] assert "pypipassword" == register_settings.password assert "pypiuser" == register_settings.username assert "/foo/bar.crt" == register_settings.cacert -def test_values_from_env_not_pypi(monkeypatch, write_config_file): +def test_values_from_env_not_pypi(monkeypatch, write_config_file, test_wheel): """Use env vars for settings when run from command line.""" write_config_file( """ @@ -131,7 +129,7 @@ def none_register(*args, **settings_kwargs): } for key, value in testenv.items(): monkeypatch.setenv(key, value) - cli.dispatch(["register", helpers.WHEEL_FIXTURE]) + cli.dispatch(["register", str(test_wheel)]) register_settings = replaced_register.calls[0].args[0] assert "pypipassword" == register_settings.password assert "someusername" == register_settings.username From 3cf48b23d4ab2ede25fd470928a25d4e26998684 Mon Sep 17 00:00:00 2001 From: Daniele Nicolodi Date: Wed, 18 Dec 2024 13:57:39 +0100 Subject: [PATCH 4/4] Move test_upload away from on-disc test fixtures The tests in this module do not care about the content of the wheel. --- tests/test_upload.py | 156 ++++++++++++++++++++++--------------------- 1 file changed, 80 insertions(+), 76 deletions(-) diff --git a/tests/test_upload.py b/tests/test_upload.py index 24916399..81aea6b9 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -22,7 +22,8 @@ from twine import package as package_file from twine.commands import upload -from . import helpers +from .helpers import build_sdist +from .helpers import build_wheel RELEASE_URL = "https://pypi.org/project/twine/1.5.0/" NEW_RELEASE_URL = "https://pypi.org/project/twine/1.6.5/" @@ -59,11 +60,13 @@ def upload_settings(make_settings, stub_repository): return upload_settings -def test_make_package_pre_signed_dist(upload_settings, caplog): +def test_make_package_pre_signed_dist( + upload_settings, caplog, test_wheel, test_wheel_signature +): """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" + filename = str(test_wheel) + expected_size = "{:.1f} KB".format(test_wheel.stat().st_size / 1024) + signed_filename = str(test_wheel_signature) signatures = {os.path.basename(signed_filename): signed_filename} upload_settings.sign = True @@ -81,10 +84,10 @@ def test_make_package_pre_signed_dist(upload_settings, caplog): ] -def test_make_package_unsigned_dist(upload_settings, monkeypatch, caplog): +def test_make_package_unsigned_dist(upload_settings, monkeypatch, caplog, test_wheel): """Create a PackageFile and print path, size, and Twine-generated signature.""" - filename = helpers.NEW_WHEEL_FIXTURE - expected_size = "21.9 KB" + filename = str(test_wheel) + expected_size = "{:.1f} KB".format(test_wheel.stat().st_size / 1024) signatures = {} upload_settings.sign = True @@ -106,29 +109,23 @@ def stub_sign(package, *_): ] -def test_make_package_attestations_flagged_but_missing(upload_settings): +def test_make_package_attestations_flagged_but_missing(upload_settings, test_wheel): """Fail when the user requests attestations but does not supply any attestations.""" upload_settings.attestations = True with pytest.raises( exceptions.InvalidDistribution, match="Upload with attestations requested" ): - upload._make_package(helpers.NEW_WHEEL_FIXTURE, {}, [], upload_settings) + upload._make_package(str(test_wheel), {}, [], upload_settings) -def test_successs_prints_release_urls(upload_settings, stub_repository, capsys): +def test_successs_prints_release_urls( + upload_settings, stub_repository, capsys, test_wheel +): """Print PyPI release URLS for each uploaded package.""" stub_repository.release_urls = lambda packages: {RELEASE_URL, NEW_RELEASE_URL} - result = upload.upload( - upload_settings, - [ - helpers.WHEEL_FIXTURE, - helpers.SDIST_FIXTURE, - helpers.NEW_SDIST_FIXTURE, - helpers.NEW_WHEEL_FIXTURE, - ], - ) + result = upload.upload(upload_settings, [str(test_wheel)]) assert result is None captured = capsys.readouterr() @@ -136,13 +133,18 @@ 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, caplog): +def test_print_packages_if_verbose(upload_settings, caplog, tmp_path): """Print the path and file size of each distribution attempting to be uploaded.""" + small_wheel = build_wheel(tmp_path, "small", "1.2.3", {"data.py": "x"}) + large_wheel = build_wheel(tmp_path, "large", "1.2.3", {"data.py": "x" * 2000}) + small_sdist = build_sdist(tmp_path, "small", "1.2.3", {"data.py": "x"}) + large_sdist = build_sdist(tmp_path, "large", "1.2.3", {"data.py": "x" * 3000}) + dists_to_upload = { - helpers.WHEEL_FIXTURE: "15.4 KB", - helpers.NEW_WHEEL_FIXTURE: "21.9 KB", - helpers.SDIST_FIXTURE: "20.8 KB", - helpers.NEW_SDIST_FIXTURE: "26.1 KB", + str(small_wheel): "{:.1f} KB".format(small_wheel.stat().st_size / 1024), + str(large_wheel): "{:.1f} KB".format(large_wheel.stat().st_size / 1024), + str(small_sdist): "{:.1f} KB".format(small_sdist.stat().st_size / 1024), + str(large_sdist): "{:.1f} KB".format(large_sdist.stat().st_size / 1024), } upload_settings.verbose = True @@ -155,13 +157,15 @@ def test_print_packages_if_verbose(upload_settings, caplog): ] -def test_print_response_if_verbose(upload_settings, stub_response, caplog): +def test_print_response_if_verbose( + upload_settings, stub_response, caplog, test_wheel, test_sdist +): """Print details about the response from the repository.""" upload_settings.verbose = True result = upload.upload( upload_settings, - [helpers.WHEEL_FIXTURE, helpers.SDIST_FIXTURE], + [str(test_wheel), str(test_sdist)], ) assert result is None @@ -173,19 +177,21 @@ def test_print_response_if_verbose(upload_settings, stub_response, caplog): assert caplog.messages.count(response_log) == 2 -def test_success_with_pre_signed_distribution(upload_settings, stub_repository, caplog): +def test_success_with_pre_signed_distribution( + upload_settings, stub_repository, caplog, test_wheel, test_wheel_signature +): """Add GPG signature provided by user to uploaded package.""" # Upload a pre-signed distribution result = upload.upload( - upload_settings, [helpers.WHEEL_FIXTURE, helpers.WHEEL_FIXTURE + ".asc"] + upload_settings, [str(test_wheel), str(test_wheel_signature)] ) assert result is None # The signature should be added via package.add_gpg_signature() package = stub_repository.upload.calls[0].args[0] assert package.gpg_signature == ( - "twine-1.5.0-py2.py3-none-any.whl.asc", - b"signature", + test_wheel_signature.name, + b"-----BEGIN PGP SIGNATURE-----", ) # Ensure that a warning is emitted. @@ -196,7 +202,7 @@ def test_success_with_pre_signed_distribution(upload_settings, stub_repository, def test_warns_potential_pgp_removal_on_3p_index( - make_settings, stub_repository, caplog + make_settings, stub_repository, caplog, test_wheel, test_wheel_signature ): """Warn when a PGP signature is specified for upload to a third-party index.""" upload_settings = make_settings( @@ -211,15 +217,15 @@ def test_warns_potential_pgp_removal_on_3p_index( # Upload a pre-signed distribution result = upload.upload( - upload_settings, [helpers.WHEEL_FIXTURE, helpers.WHEEL_FIXTURE + ".asc"] + upload_settings, [str(test_wheel), str(test_wheel_signature)] ) assert result is None # The signature should be added via package.add_gpg_signature() package = stub_repository.upload.calls[0].args[0] assert package.gpg_signature == ( - "twine-1.5.0-py2.py3-none-any.whl.asc", - b"signature", + test_wheel_signature.name, + b"-----BEGIN PGP SIGNATURE-----", ) # Ensure that a warning is emitted. @@ -231,11 +237,13 @@ def test_warns_potential_pgp_removal_on_3p_index( ) -def test_exception_with_only_pre_signed_file(upload_settings, stub_repository): +def test_exception_with_only_pre_signed_file( + upload_settings, stub_repository, test_wheel_signature +): """Raise an exception when only a signed file is uploaded.""" # Upload only pre-signed file with pytest.raises(exceptions.InvalidDistribution) as err: - upload.upload(upload_settings, [helpers.WHEEL_FIXTURE + ".asc"]) + upload.upload(upload_settings, [str(test_wheel_signature)]) assert ( "Cannot upload signed files by themselves, must upload with a " @@ -243,10 +251,11 @@ def test_exception_with_only_pre_signed_file(upload_settings, stub_repository): ) -def test_success_when_gpg_is_run(upload_settings, stub_repository, monkeypatch): +def test_success_when_gpg_is_run( + upload_settings, stub_repository, monkeypatch, test_wheel, test_wheel_signature +): """Add GPG signature generated by gpg command to uploaded package.""" - # Indicate that upload() should run_gpg() to generate the signature, which - # we'll stub out to use WHEEL_FIXTURE + ".asc" + # Indicate that upload() should run_gpg() to generate the signature upload_settings.sign = True upload_settings.sign_with = "gpg" monkeypatch.setattr( @@ -256,22 +265,22 @@ def test_success_when_gpg_is_run(upload_settings, stub_repository, monkeypatch): ) # Upload an unsigned distribution - result = upload.upload(upload_settings, [helpers.WHEEL_FIXTURE]) + result = upload.upload(upload_settings, [str(test_wheel)]) assert result is None # The signature should be added via package.sign() package = stub_repository.upload.calls[0].args[0] assert len(package.run_gpg.calls) == 1 - assert helpers.WHEEL_FIXTURE in package.run_gpg.calls[0].args[1] + assert str(test_wheel) in package.run_gpg.calls[0].args[1] assert package.gpg_signature == ( - "twine-1.5.0-py2.py3-none-any.whl.asc", - b"signature", + test_wheel_signature.name, + b"-----BEGIN PGP SIGNATURE-----", ) @pytest.mark.parametrize("verbose", [False, True]) def test_exception_for_http_status( - verbose, upload_settings, stub_response, capsys, caplog + verbose, upload_settings, stub_response, capsys, caplog, test_wheel ): upload_settings.verbose = verbose @@ -282,14 +291,14 @@ def test_exception_for_http_status( stub_response.raise_for_status = pretend.raiser(requests.HTTPError) with pytest.raises(requests.HTTPError): - upload.upload(upload_settings, [helpers.WHEEL_FIXTURE]) + upload.upload(upload_settings, [str(test_wheel)]) captured = capsys.readouterr() assert RELEASE_URL not in captured.out if verbose: assert caplog.messages == [ - f"{helpers.WHEEL_FIXTURE} (15.4 KB)", + f"{str(test_wheel)} ({test_wheel.stat().st_size / 1024:.1f} KB)", f"Response from {stub_response.url}:\n403 {stub_response.reason}", stub_response.text, ] @@ -320,7 +329,7 @@ def test_get_config_old_format(make_settings, config_file): ) -def test_deprecated_repo(make_settings): +def test_deprecated_repo(make_settings, test_wheel): with pytest.raises(exceptions.UploadToDeprecatedPyPIDetected) as err: upload_settings = make_settings( """ @@ -331,7 +340,7 @@ def test_deprecated_repo(make_settings): """ ) - upload.upload(upload_settings, [helpers.WHEEL_FIXTURE]) + upload.upload(upload_settings, [str(test_wheel)]) assert all( text in err.value.args[0] @@ -370,6 +379,7 @@ def test_exception_for_redirect( redirect_url, message_match, make_settings, + test_wheel, ): # Not using fixtures because this setup is significantly different @@ -398,31 +408,30 @@ def test_exception_for_redirect( upload_settings.create_repository = lambda: stub_repository with pytest.raises(exceptions.RedirectDetected, match=message_match): - upload.upload(upload_settings, [helpers.WHEEL_FIXTURE]) + upload.upload(upload_settings, [str(test_wheel)]) def test_prints_skip_message_for_uploaded_package( - upload_settings, stub_repository, capsys, caplog + upload_settings, stub_repository, capsys, caplog, test_wheel ): upload_settings.skip_existing = True # Short-circuit the upload stub_repository.package_is_uploaded = lambda package: True - result = upload.upload(upload_settings, [helpers.WHEEL_FIXTURE]) + result = upload.upload(upload_settings, [str(test_wheel)]) assert result is None captured = capsys.readouterr() assert RELEASE_URL not in captured.out assert caplog.messages == [ - "Skipping twine-1.5.0-py2.py3-none-any.whl " - "because it appears to already exist" + f"Skipping {test_wheel.name} because it appears to already exist" ] def test_prints_skip_message_for_response( - upload_settings, stub_response, stub_repository, capsys, caplog + upload_settings, stub_response, stub_repository, capsys, caplog, test_wheel ): upload_settings.skip_existing = True @@ -433,15 +442,14 @@ def test_prints_skip_message_for_response( # Do the upload, triggering the error response stub_repository.package_is_uploaded = lambda package: False - result = upload.upload(upload_settings, [helpers.WHEEL_FIXTURE]) + result = upload.upload(upload_settings, [str(test_wheel)]) assert result is None captured = capsys.readouterr() assert RELEASE_URL not in captured.out assert caplog.messages == [ - "Skipping twine-1.5.0-py2.py3-none-any.whl " - "because it appears to already exist" + f"Skipping {test_wheel.name} because it appears to already exist" ] @@ -522,11 +530,11 @@ def test_prints_skip_message_for_response( ), ], ) -def test_skip_existing_skips_files_on_repository(response_kwargs): +def test_skip_existing_skips_files_on_repository(response_kwargs, test_wheel): assert upload.skip_upload( response=pretend.stub(**response_kwargs), skip_existing=True, - package=package_file.PackageFile.from_filename(helpers.WHEEL_FIXTURE, None), + package=package_file.PackageFile.from_filename(str(test_wheel), None), ) @@ -539,19 +547,19 @@ def test_skip_existing_skips_files_on_repository(response_kwargs): pytest.param(dict(status_code=404), id="wrong_code"), ], ) -def test_skip_upload_doesnt_match(response_kwargs): +def test_skip_upload_doesnt_match(response_kwargs, test_wheel): assert not upload.skip_upload( response=pretend.stub(**response_kwargs), skip_existing=True, - package=package_file.PackageFile.from_filename(helpers.WHEEL_FIXTURE, None), + package=package_file.PackageFile.from_filename(str(test_wheel), None), ) -def test_skip_upload_respects_skip_existing(): +def test_skip_upload_respects_skip_existing(test_wheel): assert not upload.skip_upload( response=pretend.stub(), skip_existing=False, - package=package_file.PackageFile.from_filename(helpers.WHEEL_FIXTURE, None), + package=package_file.PackageFile.from_filename(str(test_wheel), None), ) @@ -615,25 +623,21 @@ def none_upload(*args, **settings_kwargs): "repo_url", ["https://upload.pypi.org/", "https://test.pypi.org/", "https://pypi.org/"], ) -def test_check_status_code_for_wrong_repo_url(repo_url, upload_settings, stub_response): +def test_check_status_code_for_wrong_repo_url( + repo_url, upload_settings, stub_response, test_wheel +): upload_settings.repository_config["repository"] = repo_url stub_response.url = repo_url stub_response.status_code = 405 with pytest.raises(exceptions.InvalidPyPIUploadURL): - upload.upload( - upload_settings, - [ - helpers.WHEEL_FIXTURE, - helpers.SDIST_FIXTURE, - helpers.NEW_SDIST_FIXTURE, - helpers.NEW_WHEEL_FIXTURE, - ], - ) + upload.upload(upload_settings, [str(test_wheel)]) -def test_upload_warns_attestations_non_pypi(upload_settings, caplog, stub_response): +def test_upload_warns_attestations_non_pypi( + upload_settings, caplog, stub_response, test_wheel +): upload_settings.repository_config["repository"] = "https://notpypi.example.com" upload_settings.attestations = True @@ -642,7 +646,7 @@ def test_upload_warns_attestations_non_pypi(upload_settings, caplog, stub_respon with pytest.raises(exceptions.InvalidDistribution): upload.upload( upload_settings, - [helpers.WHEEL_FIXTURE, helpers.WHEEL_FIXTURE + ".foo.attestation"], + [str(test_wheel), str(test_wheel) + ".foo.attestation"], ) assert (