From 749f782358cb2edc321668e40b1616ffb2556b16 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Sat, 28 Nov 2020 07:00:19 -0800 Subject: [PATCH] Avoid spurious output during tests 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. --- tests/conftest.py | 20 ++++++++++++++++---- tests/test_cli_compile.py | 2 +- tests/test_repository_local.py | 21 +++++++++++++++++++-- tests/test_repository_pypi.py | 8 +++++++- tests/test_sync.py | 31 +++++++++++++++++++------------ tests/test_utils.py | 16 +++++++++++++--- tests/test_writer.py | 11 +++++++++-- 7 files changed, 84 insertions(+), 25 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 2ee8fc525..e96b621f5 100644 --- a/tests/conftest.py +++ b/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 @@ -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( @@ -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 @@ -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 diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index cca7ecbd6..b48263bca 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -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 diff --git a/tests/test_repository_local.py b/tests/test_repository_local.py index b7ed1ea65..baf51aae8 100644 --- a/tests/test_repository_local.py +++ b/tests/test_repository_local.py @@ -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): @@ -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]}} @@ -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): diff --git a/tests/test_repository_pypi.py b/tests/test_repository_pypi.py index 686ea60c5..b0b74d6c8 100644 --- a/tests/test_repository_pypi.py +++ b/tests/test_repository_pypi.py @@ -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", @@ -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 diff --git a/tests/test_sync.py b/tests/test_sync.py index 06b0defbb..f1ea97613 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -420,9 +420,8 @@ 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 @@ -430,33 +429,41 @@ def test_sync_ask_declined( 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") diff --git a/tests/test_utils.py b/tests/test_utils.py index 8f876143f..71ea1e91a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,6 +1,7 @@ # coding: utf-8 from __future__ import unicode_literals +import logging import os import pytest @@ -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" diff --git a/tests/test_writer.py b/tests/test_writer.py index 4abbdffcb..3e0b37a32 100644 --- a/tests/test_writer.py +++ b/tests/test_writer.py @@ -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, @@ -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")] @@ -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")] @@ -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):