Skip to content

Commit

Permalink
Avoid spurious output during tests
Browse files Browse the repository at this point in the history
It is often convenient to use the pytest option "-s" (shortcut for
--capture=no) to view one's own debugging print() output. When there is
already lots of spurious output, it produces lots of noise and it may be
difficult to view the intended debugging output. By avoiding unnecessary
output, it is easier to find.

Tests that have intentional output now assert that output. For example,
the output of the sync command is now asserted. In addition to the
advantage above, this creates a more robust test suite as the expected
behavior is now more explicit, precise, and better covered.
  • Loading branch information
jdufresne committed Dec 3, 2020
1 parent fd81ee0 commit 7cd9d58
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 25 deletions.
20 changes: 16 additions & 4 deletions tests/conftest.py
@@ -1,9 +1,9 @@
import json
import os
import subprocess
import sys
from contextlib import contextmanager
from functools import partial
from subprocess import check_call
from textwrap import dedent

import pytest
Expand Down Expand Up @@ -259,6 +259,9 @@ def _make_package(name, version="0.1", install_requires=None):
setup(
name={name!r},
version={version!r},
author="pip-tools",
author_email="pip-tools@localhost",
url="https://github.com/jazzband/pip-tools",
install_requires={install_requires_str},
)
""".format(
Expand All @@ -268,6 +271,12 @@ def _make_package(name, version="0.1", install_requires=None):
)
)
)

# Create a README to avoid setuptools warnings.
readme_file = str(package_dir / "README")
with open(readme_file, "w"):
pass

return package_dir

return _make_package
Expand All @@ -281,9 +290,12 @@ def run_setup_file():

def _run_setup_file(package_dir_path, *args):
setup_file = str(package_dir_path / "setup.py")
return check_call(
(sys.executable, setup_file) + args, cwd=str(package_dir_path)
) # nosec
with open(os.devnull, "w") as fp:
return subprocess.check_call(
(sys.executable, setup_file) + args,
cwd=str(package_dir_path),
stdout=fp,
) # nosec

return _run_setup_file

Expand Down
2 changes: 1 addition & 1 deletion tests/test_cli_compile.py
Expand Up @@ -311,7 +311,7 @@ def test_realistic_complex_sub_dependencies(runner):
with open("requirements.in", "w") as req_in:
req_in.write("fake_with_deps") # require fake package

out = runner.invoke(cli, ["-v", "-n", "--rebuild", "-f", wheels_dir])
out = runner.invoke(cli, ["-n", "--rebuild", "-f", wheels_dir])

assert out.exit_code == 0

Expand Down
21 changes: 19 additions & 2 deletions tests/test_repository_local.py
Expand Up @@ -9,12 +9,20 @@
EXPECTED = {"sha256:5e6071ee6e4c59e0d0408d366fe9b66781d2cf01be9a6e19a2433bb3c5336330"}


def test_get_hashes_local_repository_cache_miss(pip_conf, from_line, pypi_repository):
def test_get_hashes_local_repository_cache_miss(
capfd, pip_conf, from_line, pypi_repository
):
existing_pins = {}
local_repository = LocalRequirementsRepository(existing_pins, pypi_repository)
with local_repository.allow_all_wheels():
hashes = local_repository.get_hashes(from_line("small-fake-a==0.1"))
assert hashes == EXPECTED
captured = capfd.readouterr()
assert captured.out == ""
assert (
captured.err.strip()
== "Couldn't get hashes from PyPI, fallback to hashing files"
)


def test_get_hashes_local_repository_cache_hit(from_line, repository):
Expand All @@ -37,7 +45,7 @@ def test_get_hashes_local_repository_cache_hit(from_line, repository):
("reuse_hashes", "expected"), ((True, NONSENSE), (False, EXPECTED))
)
def test_toggle_reuse_hashes_local_repository(
pip_conf, from_line, pypi_repository, reuse_hashes, expected
capfd, pip_conf, from_line, pypi_repository, reuse_hashes, expected
):
# Create an install requirement with the hashes included in its options
options = {"hashes": {"sha256": [entry.split(":")[1] for entry in NONSENSE]}}
Expand All @@ -49,6 +57,15 @@ def test_toggle_reuse_hashes_local_repository(
)
with local_repository.allow_all_wheels():
assert local_repository.get_hashes(from_line("small-fake-a==0.1")) == expected
captured = capfd.readouterr()
assert captured.out == ""
if reuse_hashes:
assert captured.err == ""
else:
assert (
captured.err.strip()
== "Couldn't get hashes from PyPI, fallback to hashing files"
)


class FakeRepositoryChecksForCopy(FakeRepository):
Expand Down
8 changes: 7 additions & 1 deletion tests/test_repository_pypi.py
Expand Up @@ -10,7 +10,7 @@
from piptools.repositories.pypi import open_local_or_remote_file


def test_generate_hashes_all_platforms(pip_conf, from_line, pypi_repository):
def test_generate_hashes_all_platforms(capfd, pip_conf, from_line, pypi_repository):
expected = {
"sha256:8d4d131cd05338e09f461ad784297efea3652e542c5fabe04a62358429a6175e",
"sha256:ad05e1371eb99f257ca00f791b755deb22e752393eb8e75bc01d651715b02ea9",
Expand All @@ -20,6 +20,12 @@ def test_generate_hashes_all_platforms(pip_conf, from_line, pypi_repository):
ireq = from_line("small-fake-multi-arch==0.1")
with pypi_repository.allow_all_wheels():
assert pypi_repository.get_hashes(ireq) == expected
captured = capfd.readouterr()
assert captured.out == ""
assert (
captured.err.strip()
== "Couldn't get hashes from PyPI, fallback to hashing files"
)


@pytest.mark.network
Expand Down
31 changes: 19 additions & 12 deletions tests/test_sync.py
Expand Up @@ -420,43 +420,50 @@ def test_sync_dry_run(runner, from_line, to_install, to_uninstall, expected_mess
),
)
@mock.patch("piptools.sync.check_call")
@mock.patch("piptools.sync.click.confirm", return_value=False)
def test_sync_ask_declined(
confirm, check_call, runner, from_line, to_install, to_uninstall, expected_message
check_call, runner, from_line, to_install, to_uninstall, expected_message
):
"""
Sync with --ask option does a dry run if the user declines
"""

to_install = set(from_line(pkg) for pkg in to_install)

with runner.isolation() as (stdout, _):
with runner.isolation("n\n") as (stdout, _):
sync(to_install, to_uninstall, ask=True)

assert stdout.getvalue().decode().splitlines() == [
expected_message,
" click==4.0",
" django==1.8",
"Would you like to proceed with these changes? [y/N]: n",
]
confirm.assert_called_once_with("Would you like to proceed with these changes?")
check_call.assert_not_called()


@pytest.mark.parametrize("dry_run", (True, False))
@mock.patch("piptools.sync.click.confirm")
@mock.patch("piptools.sync.check_call")
def test_sync_ask_accepted(check_call, confirm, from_line, dry_run):
def test_sync_ask_accepted(check_call, runner, from_line, dry_run):
"""
pip should be called as normal when the user confirms, even with dry_run
"""
confirm.return_value = True

sync(
{from_line("django==1.8")}, {from_line("click==4.0")}, ask=True, dry_run=dry_run
)
assert check_call.call_count == 2
with runner.isolation("y\n") as (stdout, _):
sync(
{from_line("django==1.8")},
{from_line("click==4.0")},
ask=True,
dry_run=dry_run,
)

confirm.assert_called_once_with("Would you like to proceed with these changes?")
assert check_call.call_count == 2
assert stdout.getvalue().decode().splitlines() == [
"Would uninstall:",
" click==4.0",
"Would install:",
" django==1.8",
"Would you like to proceed with these changes? [y/N]: y",
]


@mock.patch("piptools.sync.check_call")
Expand Down
16 changes: 13 additions & 3 deletions tests/test_utils.py
@@ -1,6 +1,7 @@
# coding: utf-8
from __future__ import unicode_literals

import logging
import os

import pytest
Expand Down Expand Up @@ -178,15 +179,24 @@ def test_is_pinned_requirement_editable(from_editable):
("https://example.com/example.zip", True),
("https://example.com/example.zip#egg=example", True),
("git+git://github.com/jazzband/pip-tools@master", True),
("../example.zip", True),
("/example.zip", True),
),
)
def test_is_url_requirement(from_line, line, expected):
def test_is_url_requirement(caplog, from_line, line, expected):
ireq = from_line(line)
assert is_url_requirement(ireq) is expected


@pytest.mark.parametrize("line", ("../example.zip", "/example.zip"))
def test_is_url_requirement_filename(caplog, from_line, line):
# Ignore warning:
#
# Requirement '../example.zip' looks like a filename, but the file does
# not exist
caplog.set_level(logging.ERROR, logger="pip")
ireq = from_line(line)
assert is_url_requirement(ireq) is True


def test_name_from_req(from_line):
ireq = from_line("django==1.8")
assert name_from_req(ireq.req) == "django"
Expand Down
11 changes: 9 additions & 2 deletions tests/test_writer.py
Expand Up @@ -5,6 +5,7 @@
from piptools.utils import comment
from piptools.writer import (
MESSAGE_UNHASHED_PACKAGE,
MESSAGE_UNINSTALLABLE,
MESSAGE_UNSAFE_PACKAGES,
MESSAGE_UNSAFE_PACKAGES_UNPINNED,
OutputWriter,
Expand Down Expand Up @@ -117,7 +118,7 @@ def test_iter_lines__unsafe_dependencies(writer, from_line, allow_unsafe):
assert tuple(lines) == expected_lines


def test_iter_lines__unsafe_with_hashes(writer, from_line):
def test_iter_lines__unsafe_with_hashes(capfd, writer, from_line):
writer.allow_unsafe = False
writer.emit_header = False
ireqs = [from_line("test==1.2")]
Expand All @@ -133,9 +134,12 @@ def test_iter_lines__unsafe_with_hashes(writer, from_line):
comment("# setuptools"),
)
assert tuple(lines) == expected_lines
captured = capfd.readouterr()
assert captured.out == ""
assert captured.err.strip() == MESSAGE_UNINSTALLABLE


def test_iter_lines__hash_missing(writer, from_line):
def test_iter_lines__hash_missing(capfd, writer, from_line):
writer.allow_unsafe = False
writer.emit_header = False
ireqs = [from_line("test==1.2"), from_line("file:///example/#egg=example")]
Expand All @@ -149,6 +153,9 @@ def test_iter_lines__hash_missing(writer, from_line):
"test==1.2 \\\n --hash=FAKEHASH",
)
assert tuple(lines) == expected_lines
captured = capfd.readouterr()
assert captured.out == ""
assert captured.err.strip() == MESSAGE_UNINSTALLABLE


def test_iter_lines__no_warn_if_only_unhashable_packages(writer, from_line):
Expand Down

0 comments on commit 7cd9d58

Please sign in to comment.