Merge pull request #488 from pradyunsg/better-check-command

Improve the check command output

docs/changelog.rst

@@ -3,6 +3,10 @@
 =========
 Changelog
 =========
+* :feature:`488` Improved output on ``check`` command:
+  Prints a message when there are no distributions given to check.
+  Improved handling of errors in a distribution's markup, avoiding
+  messages flowing through to the next distribution's errors.
 * :release:`1.14.0 <2019-09-06>`
 * :feature:`456` Better error handling and gpg2 fallback if gpg not available.
 * :bug:`341` Fail more gracefully when encountering bad metadata

tests/test_check.py

@@ -18,31 +18,31 @@
 from twine.commands import check
 
 
-def test_warningstream_write_match():
-    stream = check._WarningStream()
-    stream.output = pretend.stub(write=pretend.call_recorder(lambda a: None))
+class TestWarningStream:
 
-    stream.write("<string>:2: (WARNING/2) Title underline too short.")
+    def setup(self):
+        self.stream = check._WarningStream()
+        self.stream.output = pretend.stub(
+            write=pretend.call_recorder(lambda a: None),
+            getvalue=lambda: "result",
+        )
 
-    assert stream.output.write.calls == [
-        pretend.call("line 2: Warning: Title underline too short.\n")
-    ]
-
-
-def test_warningstream_write_nomatch():
-    stream = check._WarningStream()
-    stream.output = pretend.stub(write=pretend.call_recorder(lambda a: None))
+    def test_write_match(self):
+        self.stream.write("<string>:2: (WARNING/2) Title underline too short.")
 
-    stream.write("this does not match")
+        assert self.stream.output.write.calls == [
+            pretend.call("line 2: Warning: Title underline too short.\n")
+        ]
 
-    assert stream.output.write.calls == [pretend.call("this does not match")]
+    def test_write_nomatch(self):
+        self.stream.write("this does not match")
 
+        assert self.stream.output.write.calls == [
+            pretend.call("this does not match")
+        ]
 
-def test_warningstream_str():
-    stream = check._WarningStream()
-    stream.output = pretend.stub(getvalue=lambda: "result")
-
-    assert str(stream) == "result"
+    def test_str_representation(self):
+        assert str(self.stream) == "result"
 
 
 def test_check_no_distributions(monkeypatch):
@@ -51,7 +51,7 @@ def test_check_no_distributions(monkeypatch):
     monkeypatch.setattr(check, "_find_dists", lambda a: [])
 
     assert not check.check("dist/*", output_stream=stream)
-    assert stream.getvalue() == ""
+    assert stream.getvalue() == "No files to check.\n"
 
 
 def test_check_passing_distribution(monkeypatch):
@@ -74,10 +74,7 @@ def test_check_passing_distribution(monkeypatch):
     monkeypatch.setattr(check, "_WarningStream", lambda: warning_stream)
 
     assert not check.check("dist/*", output_stream=output_stream)
-    assert (
-        output_stream.getvalue()
-        == "Checking distribution dist/dist.tar.gz: Passed\n"
-    )
+    assert output_stream.getvalue() == "Checking dist/dist.tar.gz: PASSED\n"
     assert renderer.render.calls == [
         pretend.call("blah", stream=warning_stream)
     ]
@@ -99,11 +96,10 @@ def test_check_no_description(monkeypatch, capsys):
     output_stream = check.StringIO()
     check.check("dist/*", output_stream=output_stream)
     assert output_stream.getvalue() == (
-        'Checking distribution dist/dist.tar.gz: '
-        'warning: `long_description_content_type` missing.  '
+        'Checking dist/dist.tar.gz: PASSED, with warnings\n'
+        '  warning: `long_description_content_type` missing.  '
         'defaulting to `text/x-rst`.\n'
-        'warning: `long_description` missing.\n'
-        'Passed\n'
+        '  warning: `long_description` missing.\n'
     )
 
 
@@ -128,10 +124,10 @@ def test_check_failing_distribution(monkeypatch):
 
     assert check.check("dist/*", output_stream=output_stream)
     assert output_stream.getvalue() == (
-        "Checking distribution dist/dist.tar.gz: Failed\n"
-        "The project's long_description has invalid markup which will not be "
-        "rendered on PyPI. The following syntax errors were detected:\n"
-        "WARNING"
+        "Checking dist/dist.tar.gz: FAILED\n"
+        "  `long_description` has syntax errors in markup and would not be "
+        "rendered on PyPI.\n"
+        "    WARNING"
     )
     assert renderer.render.calls == [
         pretend.call("blah", stream=warning_stream)

twine/commands/check.py

@@ -68,47 +68,80 @@ def __str__(self):
         return self.output.getvalue()
 
 
+def _check_file(filename, render_warning_stream):
+    """Check given distribution."""
+    warnings = []
+    is_ok = True
+
+    package = PackageFile.from_filename(filename, comment=None)
+
+    metadata = package.metadata_dictionary()
+    description = metadata["description"]
+    description_content_type = metadata["description_content_type"]
+
+    if description_content_type is None:
+        warnings.append(
+            '`long_description_content_type` missing.  '
+            'defaulting to `text/x-rst`.'
+        )
+        description_content_type = 'text/x-rst'
+
+    content_type, params = cgi.parse_header(description_content_type)
+    renderer = _RENDERERS.get(content_type, _RENDERERS[None])
+
+    if description in {None, 'UNKNOWN\n\n\n'}:
+        warnings.append('`long_description` missing.')
+    elif renderer:
+        rendering_result = renderer.render(
+            description, stream=render_warning_stream, **params
+        )
+        if rendering_result is None:
+            is_ok = False
+
+    return warnings, is_ok
+
+
+# TODO: Replace with textwrap.indent when Python 2 support is dropped
+def _indented(text, prefix):
+    """Adds 'prefix' to all non-empty lines on 'text'."""
+    def prefixed_lines():
+        for line in text.splitlines(True):
+            yield (prefix + line if line.strip() else line)
+    return ''.join(prefixed_lines())
+
+
 def check(dists, output_stream=sys.stdout):
     uploads = [i for i in _find_dists(dists) if not i.endswith(".asc")]
-    stream = _WarningStream()
+    if not uploads:  # Return early, if there are no files to check.
+        output_stream.write("No files to check.\n")
+        return False
+
     failure = False
 
     for filename in uploads:
-        output_stream.write("Checking distribution %s: " % filename)
-        package = PackageFile.from_filename(filename, comment=None)
-
-        metadata = package.metadata_dictionary()
-        description = metadata["description"]
-        description_content_type = metadata["description_content_type"]
-
-        if description_content_type is None:
-            output_stream.write(
-                'warning: `long_description_content_type` missing.  '
-                'defaulting to `text/x-rst`.\n'
+        output_stream.write("Checking %s: " % filename)
+        render_warning_stream = _WarningStream()
+        warnings, is_ok = _check_file(filename, render_warning_stream)
+
+        # Print the status and/or error
+        if not is_ok:
+            failure = True
+            output_stream.write("FAILED\n")
+
+            error_text = (
+                "`long_description` has syntax errors in markup and "
+                "would not be rendered on PyPI.\n"
             )
-            description_content_type = 'text/x-rst'
-
-        content_type, params = cgi.parse_header(description_content_type)
-        renderer = _RENDERERS.get(content_type, _RENDERERS[None])
-
-        if description in {None, 'UNKNOWN\n\n\n'}:
-            output_stream.write('warning: `long_description` missing.\n')
-            output_stream.write("Passed\n")
+            output_stream.write(_indented(error_text, "  "))
+            output_stream.write(_indented(str(render_warning_stream), "    "))
+        elif warnings:
+            output_stream.write("PASSED, with warnings\n")
         else:
-            if (
-                renderer
-                and renderer.render(description, stream=stream, **params)
-                is None
-            ):
-                failure = True
-                output_stream.write("Failed\n")
-                output_stream.write(
-                    "The project's long_description has invalid markup which "
-                    "will not be rendered on PyPI. The following syntax "
-                    "errors were detected:\n%s" % stream
-                )
-            else:
-                output_stream.write("Passed\n")
+            output_stream.write("PASSED\n")
+
+        # Print warnings after the status and/or error
+        for message in warnings:
+            output_stream.write('  warning: ' + message + '\n')
 
     return failure