Make missing long_description check more flexible (#887)

* 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
pypa/readme_renderer#231 (comment)

* Rewrite tests using `build`

* Attempt to detect missing description on Windows

* Reconfigure output before each test

* Add changelong entry

Author: bhrutledge

changelog/887.bugfix.rst

@@ -0,0 +1 @@
+Restore warning for missing ``long_description``.

tests/conftest.py

@@ -1,12 +1,45 @@
 import getpass
+import logging.config
 import textwrap
 
 import pytest
+import rich
 
 from twine import settings
 from twine import utils
 
 
+@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):
     path = tmpdir / ".pypirc"

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
 
 
@@ -38,10 +38,8 @@ def test_str_representation(self):
         assert str(self.stream) == "line 2: Warning: Title underline too short."
 
 
-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",
@@ -51,75 +49,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
 
-
-def test_check_no_description(monkeypatch, capsys, caplog):
-    package = pretend.stub(
-        metadata_dictionary=lambda: {
-            "description": None,
-            "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",
@@ -134,71 +108,167 @@ def test_check_no_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):