From cd9e29f435761259169098cfad88a175a9c29605 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 26 Mar 2022 06:19:17 -0400 Subject: [PATCH 1/5] Make missing long_description check more flexible. A missing `long_description` results in `UNKNOWN` being written to PKG-INFO, but at some point, the number of trailing newlines changed. See https://github.com/pypa/readme_renderer/pull/231#discussion_r830531638 --- tests/test_check.py | 5 +++-- twine/commands/check.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_check.py b/tests/test_check.py index 66809ccf..cd722adc 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -107,10 +107,11 @@ def test_check_passing_distribution_with_none_renderer( assert capsys.readouterr().out == "Checking dist/dist.tar.gz: PASSED\n" -def test_check_no_description(monkeypatch, capsys, caplog): +@pytest.mark.parametrize("description", [None, "UNKNOWN\n\n", "UNKNOWN\n\n\n"]) +def test_check_no_description(description, monkeypatch, capsys, caplog): package = pretend.stub( metadata_dictionary=lambda: { - "description": None, + "description": description, "description_content_type": None, } ) diff --git a/twine/commands/check.py b/twine/commands/check.py index d4f0f25f..faefeb69 100644 --- a/twine/commands/check.py +++ b/twine/commands/check.py @@ -93,7 +93,7 @@ def _check_file( content_type, params = cgi.parse_header(description_content_type) renderer = _RENDERERS.get(content_type, _RENDERERS[None]) - if description in {None, "UNKNOWN\n\n\n"}: + if description is None or description.rstrip("\n") == "UNKNOWN": warnings.append("`long_description` missing.") elif renderer: rendering_result = renderer.render( From 8cfce78704a072788fb91f0e06f3f944d18086e5 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 26 Mar 2022 16:21:54 -0400 Subject: [PATCH 2/5] Rewrite tests using `build` --- tests/conftest.py | 9 ++ tests/test_check.py | 291 +++++++++++++++++++++++++++----------------- tox.ini | 1 + 3 files changed, 190 insertions(+), 111 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 479b241a..fc1d217c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,11 +2,20 @@ import textwrap import pytest +import rich from twine import settings from twine import utils +def pytest_configure(config): + # Disable color codes and wrapping during tests + rich.reconfigure( + no_color=True, + width=500, + ) + + @pytest.fixture() def config_file(tmpdir, monkeypatch): path = tmpdir / ".pypirc" diff --git a/tests/test_check.py b/tests/test_check.py index cd722adc..7cc0b9ab 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -12,12 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +import textwrap +import build import pretend import pytest -from twine import commands -from twine import package as package_file from twine.commands import check @@ -45,10 +45,8 @@ def test_str_representation(self): assert str(self.stream) == "result" -def test_check_no_distributions(monkeypatch, caplog): - monkeypatch.setattr(commands, "_find_dists", lambda a: []) - - assert not check.check(["dist/*"]) +def test_fails_no_distributions(caplog): + assert not check.check([]) assert caplog.record_tuples == [ ( "twine.commands.check", @@ -58,76 +56,51 @@ def test_check_no_distributions(monkeypatch, caplog): ] -def test_check_passing_distribution(monkeypatch, capsys): - renderer = pretend.stub(render=pretend.call_recorder(lambda *a, **kw: "valid")) - package = pretend.stub( - metadata_dictionary=lambda: { - "description": "blah", - "description_content_type": "text/markdown", - } - ) - warning_stream = "" - - monkeypatch.setattr(check, "_RENDERERS", {None: renderer}) - monkeypatch.setattr(commands, "_find_dists", lambda a: ["dist/dist.tar.gz"]) - monkeypatch.setattr( - package_file, - "PackageFile", - pretend.stub(from_filename=lambda *a, **kw: package), - ) - monkeypatch.setattr(check, "_WarningStream", lambda: warning_stream) - - assert not check.check(["dist/*"]) - assert capsys.readouterr().out == "Checking dist/dist.tar.gz: PASSED\n" - assert renderer.render.calls == [pretend.call("blah", stream=warning_stream)] - - -@pytest.mark.parametrize("content_type", ["text/plain", "text/markdown"]) -def test_check_passing_distribution_with_none_renderer( - content_type, - monkeypatch, - capsys, -): - """Pass when rendering a content type can't fail.""" - package = pretend.stub( - metadata_dictionary=lambda: { - "description": "blah", - "description_content_type": content_type, - } - ) +def build_sdist(src_path, project_files): + """ + Build a source distribution similar to `python3 -m build --sdist`. - monkeypatch.setattr(commands, "_find_dists", lambda a: ["dist/dist.tar.gz"]) - monkeypatch.setattr( - package_file, - "PackageFile", - pretend.stub(from_filename=lambda *a, **kw: package), + Returns the absolute path of the built distribution. + """ + project_files = { + "pyproject.toml": ( + """ + [build-system] + requires = ["setuptools"] + build-backend = "setuptools.build_meta" + """ + ), + **project_files, + } + + for filename, content in project_files.items(): + (src_path / filename).write_text(textwrap.dedent(content)) + + builder = build.ProjectBuilder(src_path) + return builder.build("sdist", str(src_path / "dist")) + + +@pytest.mark.parametrize("strict", [False, True]) +def test_warns_missing_description(strict, tmp_path, capsys, caplog): + sdist = build_sdist( + tmp_path, + { + "setup.cfg": ( + """ + [metadata] + name = test-package + version = 0.0.1 + """ + ), + }, ) - assert not check.check(["dist/*"]) - assert capsys.readouterr().out == "Checking dist/dist.tar.gz: PASSED\n" + assert check.check([sdist], strict=strict) is strict - -@pytest.mark.parametrize("description", [None, "UNKNOWN\n\n", "UNKNOWN\n\n\n"]) -def test_check_no_description(description, monkeypatch, capsys, caplog): - package = pretend.stub( - metadata_dictionary=lambda: { - "description": description, - "description_content_type": None, - } - ) - - monkeypatch.setattr(commands, "_find_dists", lambda a: ["dist/dist.tar.gz"]) - monkeypatch.setattr( - package_file, - "PackageFile", - pretend.stub(from_filename=lambda *a, **kw: package), + assert capsys.readouterr().out == f"Checking {sdist}: " + ( + "FAILED due to warnings\n" if strict else "PASSED with warnings\n" ) - assert not check.check(["dist/*"]) - - assert capsys.readouterr().out == ( - "Checking dist/dist.tar.gz: PASSED with warnings\n" - ) assert caplog.record_tuples == [ ( "twine.commands.check", @@ -142,71 +115,167 @@ def test_check_no_description(description, monkeypatch, capsys, caplog): ] -def test_strict_fails_on_warnings(monkeypatch, capsys, caplog): - package = pretend.stub( - metadata_dictionary=lambda: { - "description": None, - "description_content_type": None, - } +def test_warns_missing_file(tmp_path, capsys, caplog): + sdist = build_sdist( + tmp_path, + { + "setup.cfg": ( + """ + [metadata] + name = test-package + version = 0.0.1 + long_description = file:README.rst + long_description_content_type = text/x-rst + """ + ), + }, ) - monkeypatch.setattr(commands, "_find_dists", lambda a: ["dist/dist.tar.gz"]) - monkeypatch.setattr( - package_file, - "PackageFile", - pretend.stub(from_filename=lambda *a, **kw: package), - ) + assert not check.check([sdist]) - assert check.check(["dist/*"], strict=True) + assert capsys.readouterr().out == f"Checking {sdist}: PASSED with warnings\n" - assert capsys.readouterr().out == ( - "Checking dist/dist.tar.gz: FAILED due to warnings\n" - ) assert caplog.record_tuples == [ ( "twine.commands.check", logging.WARNING, - "`long_description_content_type` missing. defaulting to `text/x-rst`.", + "`long_description` missing.", ), + ] + + +def test_fails_rst_syntax_error(tmp_path, capsys, caplog): + sdist = build_sdist( + tmp_path, + { + "setup.cfg": ( + """ + [metadata] + name = test-package + version = 0.0.1 + long_description = file:README.rst + long_description_content_type = text/x-rst + """ + ), + "README.rst": ( + """ + ============ + """ + ), + }, + ) + + assert check.check([sdist]) + + assert capsys.readouterr().out == f"Checking {sdist}: FAILED\n" + + assert caplog.record_tuples == [ ( "twine.commands.check", - logging.WARNING, - "`long_description` missing.", + logging.ERROR, + "`long_description` has syntax errors in markup " + "and would not be rendered on PyPI.\n" + "line 2: Error: Document or section may not begin with a transition.", ), ] -def test_check_failing_distribution(monkeypatch, capsys, caplog): - renderer = pretend.stub(render=pretend.call_recorder(lambda *a, **kw: None)) - package = pretend.stub( - metadata_dictionary=lambda: { - "description": "blah", - "description_content_type": "text/markdown", - } +def test_fails_rst_no_content(tmp_path, capsys, caplog): + sdist = build_sdist( + tmp_path, + { + "setup.cfg": ( + """ + [metadata] + name = test-package + version = 0.0.1 + long_description = file:README.rst + long_description_content_type = text/x-rst + """ + ), + "README.rst": ( + """ + test-package + ============ + """ + ), + }, ) - warning_stream = "Syntax error" - - monkeypatch.setattr(check, "_RENDERERS", {None: renderer}) - monkeypatch.setattr(commands, "_find_dists", lambda a: ["dist/dist.tar.gz"]) - monkeypatch.setattr( - package_file, - "PackageFile", - pretend.stub(from_filename=lambda *a, **kw: package), - ) - monkeypatch.setattr(check, "_WarningStream", lambda: warning_stream) - assert check.check(["dist/*"]) + assert check.check([sdist]) + + assert capsys.readouterr().out == f"Checking {sdist}: FAILED\n" - assert capsys.readouterr().out == "Checking dist/dist.tar.gz: FAILED\n" assert caplog.record_tuples == [ ( "twine.commands.check", logging.ERROR, - "`long_description` has syntax errors in markup and would not be rendered " - "on PyPI.\nSyntax error", + "`long_description` has syntax errors in markup " + "and would not be rendered on PyPI.\n", ), ] - assert renderer.render.calls == [pretend.call("blah", stream=warning_stream)] + + +def test_passes_rst_description(tmp_path, capsys, caplog): + sdist = build_sdist( + tmp_path, + { + "setup.cfg": ( + """ + [metadata] + name = test-package + version = 0.0.1 + long_description = file:README.rst + long_description_content_type = text/x-rst + """ + ), + "README.rst": ( + """ + test-package + ============ + + A test package. + """ + ), + }, + ) + + assert not check.check([sdist]) + + assert capsys.readouterr().out == f"Checking {sdist}: PASSED\n" + + assert not caplog.record_tuples + + +@pytest.mark.parametrize("content_type", ["text/markdown", "text/plain"]) +def test_passes_markdown_description(content_type, tmp_path, capsys, caplog): + sdist = build_sdist( + tmp_path, + { + "setup.cfg": ( + f""" + [metadata] + name = test-package + version = 0.0.1 + long_description = file:README.md + long_description_content_type = {content_type} + """ + ), + "README.md": ( + """ + # test-package + + A test package. + """ + ), + }, + ) + + assert not check.check([sdist]) + + assert capsys.readouterr().out == f"Checking {sdist}: PASSED\n" + + assert not caplog.record_tuples def test_main(monkeypatch): diff --git a/tox.ini b/tox.ini index 65cba7d2..fb8f6b9b 100644 --- a/tox.ini +++ b/tox.ini @@ -9,6 +9,7 @@ deps = pytest pytest-cov pytest-socket + build passenv = PYTEST_ADDOPTS commands = From 5e81076b841c8860bac0b707a4463e9e13f3ea38 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sat, 26 Mar 2022 21:38:27 -0400 Subject: [PATCH 3/5] Attempt to detect missing description on Windows --- twine/commands/check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/twine/commands/check.py b/twine/commands/check.py index faefeb69..57ff90fb 100644 --- a/twine/commands/check.py +++ b/twine/commands/check.py @@ -93,7 +93,7 @@ def _check_file( content_type, params = cgi.parse_header(description_content_type) renderer = _RENDERERS.get(content_type, _RENDERERS[None]) - if description is None or description.rstrip("\n") == "UNKNOWN": + if description is None or description.rstrip() == "UNKNOWN": warnings.append("`long_description` missing.") elif renderer: rendering_result = renderer.render( From b7a38d9d4537755f1bc0e90771a50471039d03a5 Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Sun, 27 Mar 2022 06:48:53 -0400 Subject: [PATCH 4/5] Reconfigure output before each test --- tests/conftest.py | 28 ++++++++++++++++++++++++++-- tests/test_upload.py | 16 ++++++++++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index fc1d217c..a0254a85 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ import getpass +import logging.config import textwrap import pytest @@ -8,13 +9,36 @@ from twine import utils -def pytest_configure(config): - # Disable color codes and wrapping during tests +@pytest.fixture(autouse=True) +def configure_output(): + """ + Disable colored output and line wrapping before each test. + + Some tests (e.g. test_main.py) will end up calling (and making assertions based on) + twine.cli.configure_output, which overrides this configuration. This fixture should + prevent that leaking into subsequent tests. + """ rich.reconfigure( no_color=True, width=500, ) + logging.config.dictConfig( + { + "version": 1, + "handlers": { + "console": { + "class": "logging.StreamHandler", + } + }, + "loggers": { + "twine": { + "handlers": ["console"], + }, + }, + } + ) + @pytest.fixture() def config_file(tmpdir, monkeypatch): diff --git a/tests/test_upload.py b/tests/test_upload.py index 6524be0f..d5cd64aa 100644 --- a/tests/test_upload.py +++ b/tests/test_upload.py @@ -337,7 +337,7 @@ def test_exception_for_redirect( def test_prints_skip_message_for_uploaded_package( - upload_settings, stub_repository, capsys + upload_settings, stub_repository, capsys, caplog ): upload_settings.skip_existing = True @@ -348,12 +348,16 @@ def test_prints_skip_message_for_uploaded_package( assert result is None captured = capsys.readouterr() - assert "Skipping twine-1.5.0-py2.py3-none-any.whl" in captured.out 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" + ] + def test_prints_skip_message_for_response( - upload_settings, stub_response, stub_repository, capsys + upload_settings, stub_response, stub_repository, capsys, caplog ): upload_settings.skip_existing = True @@ -368,9 +372,13 @@ def test_prints_skip_message_for_response( assert result is None captured = capsys.readouterr() - assert "Skipping twine-1.5.0-py2.py3-none-any.whl" in captured.out 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" + ] + @pytest.mark.parametrize( "response_kwargs", From 5d724681c81c77e7abc7db92acb394562efa828e Mon Sep 17 00:00:00 2001 From: Brian Rutledge Date: Wed, 30 Mar 2022 05:47:46 -0400 Subject: [PATCH 5/5] Add changelong entry --- changelog/887.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/887.bugfix.rst diff --git a/changelog/887.bugfix.rst b/changelog/887.bugfix.rst new file mode 100644 index 00000000..6cba3eee --- /dev/null +++ b/changelog/887.bugfix.rst @@ -0,0 +1 @@ +Restore warning for missing ``long_description``.