Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow "twine check" to run multiple checks #843

Closed
wants to merge 1 commit into from
Closed

Allow "twine check" to run multiple checks #843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions docs/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,11 @@ Uploads one or more distributions to a repository.
``twine check``
^^^^^^^^^^^^^^^

Checks whether your distribution's long description will render correctly on
PyPI.
Runs a series of checks on the given distribution files:

* Checks whether your distribution's long description will render correctly on
PyPI.
* Checks whether a license was specified.

.. program-output:: twine check -h

Expand Down
3 changes: 3 additions & 0 deletions setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,8 @@ twine.registered_commands =
check = twine.commands.check:main
upload = twine.commands.upload:main
register = twine.commands.register:main
twine.registered_checkers =
longdesc = twine.commands.check:check_long_description
license = twine.commands.check:check_license
Comment on lines +55 to +57
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the value in defining an entrypoint for registered_checkers? At first glance, it feels like an extra layer of complexity. Why not just call the check_* functions explicitly in twine.commands.check._check_file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, calling the check_* functions directly would work as well. My goal here was to allow users to define project-specific checkers that may not be useful to all Python developers, or are far too strict to be included in twine itself.

console_scripts =
twine = twine.__main__:main
322 changes: 177 additions & 145 deletions tests/test_check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,148 +45,180 @@ def test_str_representation(self):
assert str(self.stream) == "result"


def test_check_no_distributions(monkeypatch):
stream = io.StringIO()

monkeypatch.setattr(commands, "_find_dists", lambda a: [])

assert not check.check(["dist/*"], output_stream=stream)
assert stream.getvalue() == "No files to check.\n"


def test_check_passing_distribution(monkeypatch):
renderer = pretend.stub(render=pretend.call_recorder(lambda *a, **kw: "valid"))
package = pretend.stub(
metadata_dictionary=lambda: {
"description": "blah",
"description_content_type": "text/markdown",
}
)
output_stream = io.StringIO()
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/*"], output_stream=output_stream)
assert output_stream.getvalue() == "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):
"""Pass when rendering a content type can't fail."""
package = pretend.stub(
metadata_dictionary=lambda: {
"description": "blah",
"description_content_type": content_type,
}
)

monkeypatch.setattr(commands, "_find_dists", lambda a: ["dist/dist.tar.gz"])
monkeypatch.setattr(
package_file,
"PackageFile",
pretend.stub(from_filename=lambda *a, **kw: package),
)

output_stream = io.StringIO()
assert not check.check(["dist/*"], output_stream=output_stream)
assert output_stream.getvalue() == "Checking dist/dist.tar.gz: PASSED\n"


def test_check_no_description(monkeypatch, capsys):
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),
)

# used to crash with `AttributeError`
output_stream = io.StringIO()
assert not check.check(["dist/*"], output_stream=output_stream)
assert output_stream.getvalue() == (
"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"
)


def test_strict_fails_on_warnings(monkeypatch, capsys):
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),
)

# used to crash with `AttributeError`
output_stream = io.StringIO()
assert check.check(["dist/*"], output_stream=output_stream, strict=True)
assert output_stream.getvalue() == (
"Checking dist/dist.tar.gz: FAILED, due to warnings\n"
" warning: `long_description_content_type` missing. "
"defaulting to `text/x-rst`.\n"
" warning: `long_description` missing.\n"
)


def test_check_failing_distribution(monkeypatch):
renderer = pretend.stub(render=pretend.call_recorder(lambda *a, **kw: None))
package = pretend.stub(
metadata_dictionary=lambda: {
"description": "blah",
"description_content_type": "text/markdown",
}
)
output_stream = io.StringIO()
warning_stream = "WARNING"

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/*"], output_stream=output_stream)
assert output_stream.getvalue() == (
"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)]


def test_main(monkeypatch):
check_result = pretend.stub()
check_stub = pretend.call_recorder(lambda a, strict=False: check_result)
monkeypatch.setattr(check, "check", check_stub)

assert check.main(["dist/*"]) == check_result
assert check_stub.calls == [pretend.call(["dist/*"], strict=False)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the instinct to refactor, but it makes for a big diff that's hard to review. I probably would have just renamed the existing test functions to include the word description.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I only realized that when viewing the diff myself :-(

The tests have been slightly modified as well, since they used to be strongly tied to the "long description checking" logic. Now some tests focus on the long description checker, and some tests focus on the infrastructure, and are checker-agnostic.

class TestCheckLicense:
def test_license_field_specified(self, monkeypatch):
package = pretend.stub(
metadata_dictionary=lambda: {
"license": "Some License v42.0",
}
)

monkeypatch.setattr(
package_file,
"PackageFile",
pretend.stub(from_filename=lambda *a, **kw: package),
)

warnings, errors = check.check_license("dist/dist.tar.gz")
assert not warnings
assert not errors

def test_license_classifier_specified(self, monkeypatch):
package = pretend.stub(
metadata_dictionary=lambda: {
"classifiers": [
"License :: OSI Approved :: Apache Software License",
],
}
)

monkeypatch.setattr(
package_file,
"PackageFile",
pretend.stub(from_filename=lambda *a, **kw: package),
)

warnings, errors = check.check_license("dist/dist.tar.gz")
assert not warnings
assert not errors

def test_no_license_specified(self, monkeypatch):
package = pretend.stub(metadata_dictionary=lambda: {})

monkeypatch.setattr(
package_file,
"PackageFile",
pretend.stub(from_filename=lambda *a, **kw: package),
)

warnings, errors = check.check_license("dist/dist.tar.gz")
assert not errors
assert warnings == [
"No license specified. Use one of the `LICENSE ::` classifiers "
"or the `license` field if no classifier is relevant."
]


class TestCheckLongDescription:
def test_passing_distribution(self, monkeypatch):
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)

warnings, errors = check.check_long_description("dist/dist.tar.gz")
assert not warnings
assert not errors
assert renderer.render.calls == [pretend.call("blah", stream=warning_stream)]

@pytest.mark.parametrize("content_type", ["text/plain", "text/markdown"])
def test_passing_distribution_with_none_renderer(
self, content_type, monkeypatch
):
"""Pass when rendering a content type can't fail."""
package = pretend.stub(
metadata_dictionary=lambda: {
"description": "blah",
"description_content_type": content_type,
}
)

monkeypatch.setattr(commands, "_find_dists", lambda a: ["dist/dist.tar.gz"])
monkeypatch.setattr(
package_file,
"PackageFile",
pretend.stub(from_filename=lambda *a, **kw: package),
)

warnings, errors = check.check_long_description("dist/dist.tar.gz")
assert not warnings
assert not errors

def test_no_description(self, monkeypatch, capsys):
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),
)

# used to crash with `AttributeError`
output_stream = io.StringIO()
assert not check.check(["dist/*"], output_stream=output_stream)
warnings, errors = check.check_long_description("dist/dist.tar.gz")
assert not errors
assert warnings == [
"`long_description_content_type` missing. defaulting to `text/x-rst`.",
"`long_description` missing.",
]

def test_rendering_failure(self, monkeypatch):
renderer = pretend.stub(render=pretend.call_recorder(lambda *a, **kw: None))
package = pretend.stub(
metadata_dictionary=lambda: {
"description": "blah",
"description_content_type": "text/markdown",
}
)
warning_stream = "WARNING"

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)
warnings, errors = check.check_long_description("dist/dist.tar.gz")
assert not warnings
assert errors == [
" `long_description` has syntax errors in markup and "
"would not be rendered on PyPI.",
" WARNING",
]


class TestCheckCommand:
def test_strict_fails_on_warnings(self, monkeypatch):
monkeypatch.setattr(commands, "_find_dists", lambda a: ["dist/dist.tar.gz"])
monkeypatch.setattr(check, "_check_file", lambda *args: (["warning"], []))
assert not check.check(["dist/dist.tar.gz"], strict=False)
assert check.check(["dist/dist.tar.gz"], strict=True)

def test_check_no_distributions(self, monkeypatch):
stream = io.StringIO()

monkeypatch.setattr(commands, "_find_dists", lambda a: [])

assert not check.check(["dist/*"], output_stream=stream)
assert stream.getvalue() == "No files to check.\n"

def test_main(self, monkeypatch):
check_result = pretend.stub()
check_stub = pretend.call_recorder(lambda a, strict=False: check_result)
monkeypatch.setattr(check, "check", check_stub)

assert check.main(["dist/*"]) == check_result
assert check_stub.calls == [pretend.call(["dist/*"], strict=False)]
Loading