From 43f0ea79dce5a845cc40f88d5b1f0e50ef45d6a3 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Sun, 22 Nov 2020 08:17:01 -0800 Subject: [PATCH 1/5] Alter test_cli_compile.py to assert full output Makes for more robust tests as the complete output is tested. Helps show and catch subtle or unexpected changes in output. --- tests/test_cli_compile.py | 87 +++++++++++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 18 deletions(-) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 9a9894a8f..c12901461 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -53,12 +53,18 @@ def test_command_line_setuptools_read(pip_conf, runner): """ ) ) - out = runner.invoke(cli) - - assert "This file is autogenerated by pip-compile" in out.stderr - assert ( - "small-fake-a==0.1 # via fake-setuptools-a (setup.py)" - in out.stderr.splitlines() + out = runner.invoke(cli, ["--no-emit-find-links"]) + + assert out.stderr == dedent( + """\ + # + # This file is autogenerated by pip-compile + # To update, run: + # + # pip-compile --no-emit-find-links + # + small-fake-a==0.1 # via fake-setuptools-a (setup.py) + """ ) # check that pip-compile generated a configuration file @@ -831,10 +837,23 @@ def test_stdin(pip_conf, runner): Test compile requirements from STDIN. """ out = runner.invoke( - cli, ["-", "--output-file", "requirements.txt", "-n"], input="small-fake-a==0.1" + cli, + ["-", "--output-file", "requirements.txt", "-n", "--no-emit-find-links"], + input="small-fake-a==0.1", ) - assert "small-fake-a==0.1 # via -r -" in out.stderr.splitlines() + assert out.stderr == dedent( + """\ + # + # This file is autogenerated by pip-compile + # To update, run: + # + # pip-compile --no-emit-find-links --output-file=requirements.txt - + # + small-fake-a==0.1 # via -r - + Dry-run, so nothing updated. + """ + ) def test_multiple_input_files_without_output_file(runner): @@ -858,12 +877,36 @@ def test_multiple_input_files_without_output_file(runner): @pytest.mark.parametrize( ("option", "expected"), ( - ( + pytest.param( "--annotate", - "small-fake-a==0.1 " - "# via -c constraints.txt, small-fake-with-deps\n", + """\ + # + # This file is autogenerated by pip-compile + # To update, run: + # + # pip-compile --no-emit-find-links + # + small-fake-a==0.1 # via -c constraints.txt, small-fake-with-deps + small-fake-with-deps==0.1 # via -r requirements.in + Dry-run, so nothing updated. + """, + id="annotate", + ), + pytest.param( + "--no-annotate", + """\ + # + # This file is autogenerated by pip-compile + # To update, run: + # + # pip-compile --no-annotate --no-emit-find-links + # + small-fake-a==0.1 + small-fake-with-deps==0.1 + Dry-run, so nothing updated. + """, + id="no annotate", ), - ("--no-annotate", "small-fake-a==0.1\n"), ), ) def test_annotate_option(pip_conf, runner, option, expected): @@ -876,9 +919,9 @@ def test_annotate_option(pip_conf, runner, option, expected): req_in.write("-c constraints.txt\n") req_in.write("small_fake_with_deps") - out = runner.invoke(cli, [option, "-n"]) + out = runner.invoke(cli, [option, "-n", "--no-emit-find-links"]) - assert expected in out.stderr + assert out.stderr == dedent(expected) assert out.exit_code == 0 @@ -1087,11 +1130,19 @@ def test_upgrade_package_doesnt_remove_annotation(pip_conf, runner): "small-fake-a==0.1 # via small-fake-with-deps\n" ) - runner.invoke(cli, ["-P", "small-fake-a"]) + runner.invoke(cli, ["-P", "small-fake-a", "--no-emit-find-links"]) with open("requirements.txt", "r") as req_txt: - assert ( - "small-fake-a==0.1 # via small-fake-with-deps" - in req_txt.read().splitlines() + assert req_txt.read() == dedent( + """\ + # + # This file is autogenerated by pip-compile + # To update, run: + # + # pip-compile --no-emit-find-links + # + small-fake-a==0.1 # via small-fake-with-deps + small-fake-with-deps==0.1 # via -r requirements.in + """ ) From edbaa514d4fe2d625ef8bf9657a6dc772831ff12 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Sun, 22 Nov 2020 08:41:53 -0800 Subject: [PATCH 2/5] Add test coverage for using both annotations and generate hashes Some of the test data exceeds the 88 width limit, so this has been bumped to 100. Black will continue to format non-string statements at a max width of 88. --- setup.cfg | 2 +- tests/test_cli_compile.py | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index a755d81d7..ccad6c27e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -63,7 +63,7 @@ markers = network: mark tests that require internet access [flake8] -max-line-length = 88 +max-line-length = 100 extend-ignore = E203 # E203 conflicts with PEP8; see https://github.com/psf/black#slices # flake8-pytest-style diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index c12901461..ee4291de6 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -745,6 +745,28 @@ def test_generate_hashes_verbose(pip_conf, runner): assert expected_verbose_text in out.stderr +@pytest.mark.network +def test_generate_hashes_with_annotations(runner): + with open("requirements.in", "w") as fp: + fp.write("six==1.15.0") + + out = runner.invoke(cli, ["--generate-hashes"]) + assert out.stderr == dedent( + """\ + # + # This file is autogenerated by pip-compile + # To update, run: + # + # pip-compile --generate-hashes + # + six==1.15.0 \\ + --hash=sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259 \\ + --hash=sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced \\ + # via -r requirements.in + """ + ) + + def test_filter_pip_markers(pip_conf, runner): """ Check that pip-compile works with pip environment markers (PEP496) From 2ba8baa5a47f50733d0918d8a40cf85c6a803811 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Sun, 22 Nov 2020 06:12:03 -0800 Subject: [PATCH 3/5] Stop writing an extra trailing slash after hashes The pip requirement does not continue on the next line so the line should not end in a trailing slash. --- piptools/writer.py | 2 +- tests/test_cli_compile.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/piptools/writer.py b/piptools/writer.py index 35a6ce18e..47060b0ef 100644 --- a/piptools/writer.py +++ b/piptools/writer.py @@ -233,7 +233,7 @@ def _format_requirement(self, ireq, marker=None, hashes=None): annotation = ", ".join(sorted(required_by)) line = "{:24}{}{}".format( line, - " \\\n " if ireq_hashes else " ", + "\n " if ireq_hashes else " ", comment("# via " + annotation), ) return line diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index ee4291de6..706829e2a 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -761,7 +761,7 @@ def test_generate_hashes_with_annotations(runner): # six==1.15.0 \\ --hash=sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259 \\ - --hash=sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced \\ + --hash=sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced # via -r requirements.in """ ) From a55babeaa1b505db29a1c4eba3bb7848449d6533 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Sun, 22 Nov 2020 07:44:21 -0800 Subject: [PATCH 4/5] Always place annotations on a new line This makes the annotation output more consistent. No longer place annotations inline when hashes aren't generated. Now, always place annotations on a new line. When the annotation comment is very long, this helps make the result more readable as it is less likely to continue far off to the right. --- README.rst | 54 ++++++++++++++++++++++++++------------- piptools/writer.py | 6 +---- tests/test_cli_compile.py | 18 ++++++++----- tests/test_writer.py | 18 ++++--------- 4 files changed, 54 insertions(+), 42 deletions(-) diff --git a/README.rst b/README.rst index 2aeda3721..f84f304ad 100644 --- a/README.rst +++ b/README.rst @@ -79,10 +79,14 @@ If you have a ``setup.py`` with ``install_requires=['django']``, then run # # pip-compile # - asgiref==3.2.3 # via django - django==3.0.3 # via my_django_project (setup.py) - pytz==2019.3 # via django - sqlparse==0.3.0 # via django + asgiref==3.2.3 + # via django + django==3.0.3 + # via my_django_project (setup.py) + pytz==2019.3 + # via django + sqlparse==0.3.0 + # via django ``pip-compile`` will produce your ``requirements.txt``, with all the Django dependencies (and all underlying dependencies) pinned. @@ -109,10 +113,14 @@ Now, run ``pip-compile requirements.in``: # # pip-compile requirements.in # - asgiref==3.2.3 # via django - django==3.0.3 # via -r requirements.in - pytz==2019.3 # via django - sqlparse==0.3.0 # via django + asgiref==3.2.3 + # via django + django==3.0.3 + # via -r requirements.in + pytz==2019.3 + # via django + sqlparse==0.3.0 + # via django And it will produce your ``requirements.txt``, with all the Django dependencies (and all underlying dependencies) pinned. @@ -225,10 +233,14 @@ generated at the top of requirements files by setting the # # ./pipcompilewrapper # - asgiref==3.2.3 # via django - django==3.0.3 # via -r requirements.in - pytz==2019.3 # via django - sqlparse==0.3.0 # via django + asgiref==3.2.3 + # via django + django==3.0.3 + # via -r requirements.in + pytz==2019.3 + # via django + sqlparse==0.3.0 + # via django Workflow for layered requirements --------------------------------- @@ -267,8 +279,10 @@ First, compile ``requirements.txt`` as usual: # # pip-compile # - django==2.1.15 # via -r requirements.in - pytz==2019.3 # via django + django==2.1.15 + # via -r requirements.in + pytz==2019.3 + # via django Now compile the dev requirements and the ``requirements.txt`` file is used as @@ -283,10 +297,14 @@ a constraint: # # pip-compile dev-requirements.in # - django-debug-toolbar==2.2 # via -r dev-requirements.in - django==2.1.15 # via -c requirements.txt, django-debug-toolbar - pytz==2019.3 # via -c requirements.txt, django - sqlparse==0.3.0 # via django-debug-toolbar + django-debug-toolbar==2.2 + # via -r dev-requirements.in + django==2.1.15 + # via -c requirements.txt, django-debug-toolbar + pytz==2019.3 + # via -c requirements.txt, django + sqlparse==0.3.0 + # via django-debug-toolbar As you can see above, even though a ``2.2`` release of Django is available, the dev requirements only include a ``2.1`` version of Django because they were diff --git a/piptools/writer.py b/piptools/writer.py index 47060b0ef..23d3445fa 100644 --- a/piptools/writer.py +++ b/piptools/writer.py @@ -231,9 +231,5 @@ def _format_requirement(self, ireq, marker=None, hashes=None): required_by.add(_comes_from_as_string(ireq)) if required_by: annotation = ", ".join(sorted(required_by)) - line = "{:24}{}{}".format( - line, - "\n " if ireq_hashes else " ", - comment("# via " + annotation), - ) + line = "{}\n {}".format(line, comment("# via " + annotation)) return line diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 706829e2a..f59774bd3 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -63,7 +63,8 @@ def test_command_line_setuptools_read(pip_conf, runner): # # pip-compile --no-emit-find-links # - small-fake-a==0.1 # via fake-setuptools-a (setup.py) + small-fake-a==0.1 + # via fake-setuptools-a (setup.py) """ ) @@ -872,7 +873,8 @@ def test_stdin(pip_conf, runner): # # pip-compile --no-emit-find-links --output-file=requirements.txt - # - small-fake-a==0.1 # via -r - + small-fake-a==0.1 + # via -r - Dry-run, so nothing updated. """ ) @@ -908,8 +910,10 @@ def test_multiple_input_files_without_output_file(runner): # # pip-compile --no-emit-find-links # - small-fake-a==0.1 # via -c constraints.txt, small-fake-with-deps - small-fake-with-deps==0.1 # via -r requirements.in + small-fake-a==0.1 + # via -c constraints.txt, small-fake-with-deps + small-fake-with-deps==0.1 + # via -r requirements.in Dry-run, so nothing updated. """, id="annotate", @@ -1162,8 +1166,10 @@ def test_upgrade_package_doesnt_remove_annotation(pip_conf, runner): # # pip-compile --no-emit-find-links # - small-fake-a==0.1 # via small-fake-with-deps - small-fake-with-deps==0.1 # via -r requirements.in + small-fake-a==0.1 + # via small-fake-with-deps + small-fake-with-deps==0.1 + # via -r requirements.in """ ) diff --git a/tests/test_writer.py b/tests/test_writer.py index ea46f84f7..2811c4eba 100644 --- a/tests/test_writer.py +++ b/tests/test_writer.py @@ -53,25 +53,21 @@ def test_format_requirement_annotation_editable(from_editable, writer): assert writer._format_requirement( ireq - ) == "-e git+git://fake.org/x/y.git#egg=y " + comment("# via xyz") + ) == "-e git+git://fake.org/x/y.git#egg=y\n " + comment("# via xyz") def test_format_requirement_annotation(from_line, writer): ireq = from_line("test==1.2") ireq.comes_from = "xyz" - assert writer._format_requirement(ireq) == "test==1.2 " + comment( - "# via xyz" - ) + assert writer._format_requirement(ireq) == "test==1.2\n " + comment("# via xyz") def test_format_requirement_annotation_lower_case(from_line, writer): ireq = from_line("Test==1.2") ireq.comes_from = "xyz" - assert writer._format_requirement(ireq) == "test==1.2 " + comment( - "# via xyz" - ) + assert writer._format_requirement(ireq) == "test==1.2\n " + comment("# via xyz") def test_format_requirement_for_primary(from_line, writer): @@ -79,9 +75,7 @@ def test_format_requirement_for_primary(from_line, writer): ireq = from_line("test==1.2") ireq.comes_from = "xyz" - assert writer._format_requirement(ireq) == "test==1.2 " + comment( - "# via xyz" - ) + assert writer._format_requirement(ireq) == "test==1.2\n " + comment("# via xyz") def test_format_requirement_for_primary_lower_case(from_line, writer): @@ -89,9 +83,7 @@ def test_format_requirement_for_primary_lower_case(from_line, writer): ireq = from_line("Test==1.2") ireq.comes_from = "xyz" - assert writer._format_requirement(ireq) == "test==1.2 " + comment( - "# via xyz" - ) + assert writer._format_requirement(ireq) == "test==1.2\n " + comment("# via xyz") def test_format_requirement_environment_marker(from_line, writer): From ad084ac3c4c49f082da74715c193aaf606a890a0 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Sun, 22 Nov 2020 06:40:30 -0800 Subject: [PATCH 5/5] Improve formatting of long "via" annotations When there is more than one "via" annotation, they are now formatted one per line. This prevents very long lines in requirements.txt output. The very long lines often scroll beyond the width of an editor, making them difficult to read. Worse, when these annotations change, diffs are hard to read as the actual difference is hidden far off to the right. By placing one annotation per line, diffs will be much more obvious. This follows the philosophy of many automatic formatters such as Black. To simplify the formatting and to make it more consistent, annotations are now always on the line after the requirement, whether hashes were generated or not. Fixes #1036 --- README.rst | 8 ++++-- piptools/writer.py | 12 ++++++-- tests/test_cli_compile.py | 59 ++++++++++++++++++++++++++++++++++++++- tests/test_writer.py | 10 +++---- 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/README.rst b/README.rst index f84f304ad..69c790daf 100644 --- a/README.rst +++ b/README.rst @@ -300,9 +300,13 @@ a constraint: django-debug-toolbar==2.2 # via -r dev-requirements.in django==2.1.15 - # via -c requirements.txt, django-debug-toolbar + # via + # -c requirements.txt + # django-debug-toolbar pytz==2019.3 - # via -c requirements.txt, django + # via + # -c requirements.txt + # django sqlparse==0.3.0 # via django-debug-toolbar diff --git a/piptools/writer.py b/piptools/writer.py index 23d3445fa..515df198e 100644 --- a/piptools/writer.py +++ b/piptools/writer.py @@ -230,6 +230,14 @@ def _format_requirement(self, ireq, marker=None, hashes=None): elif ireq.comes_from: required_by.add(_comes_from_as_string(ireq)) if required_by: - annotation = ", ".join(sorted(required_by)) - line = "{}\n {}".format(line, comment("# via " + annotation)) + required_by = sorted(required_by) + if len(required_by) == 1: + source = required_by[0] + annotation = " # via " + source + else: + annotation_lines = [" # via"] + for source in required_by: + annotation_lines.append(" # " + source) + annotation = "\n".join(annotation_lines) + line = "{}\n{}".format(line, comment(annotation)) return line diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index f59774bd3..cca7ecbd6 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -768,6 +768,61 @@ def test_generate_hashes_with_annotations(runner): ) +@pytest.mark.network +def test_generate_hashes_with_long_annotations(runner): + with open("requirements.in", "w") as fp: + fp.write("django-debug-toolbar==1.11\n") + fp.write("django-storages==1.9.1\n") + fp.write("django-taggit==0.24.0\n") + fp.write("Django==1.11.29\n") + fp.write("pytz==2020.4\n") + fp.write("sqlparse==0.3.1\n") + + out = runner.invoke(cli, ["--generate-hashes"]) + assert out.stderr == dedent( + """\ + # + # This file is autogenerated by pip-compile + # To update, run: + # + # pip-compile --generate-hashes + # + django-debug-toolbar==1.11 \\ + --hash=sha256:89d75b60c65db363fb24688d977e5fbf0e73386c67acf562d278402a10fc3736 \\ + --hash=sha256:c2b0134119a624f4ac9398b44f8e28a01c7686ac350a12a74793f3dd57a9eea0 + # via -r requirements.in + django-storages==1.9.1 \\ + --hash=sha256:3103991c2ee8cef8a2ff096709973ffe7106183d211a79f22cf855f33533d924 \\ + --hash=sha256:a59e9923cbce7068792f75344ed7727021ee4ac20f227cf17297d0d03d141e91 + # via -r requirements.in + django-taggit==0.24.0 \\ + --hash=sha256:710b4d15ec1996550cc68a0abbc41903ca7d832540e52b1336e6858737e410d8 \\ + --hash=sha256:bb8f27684814cd1414b2af75b857b5e26a40912631904038a7ecacd2bfafc3ac + # via -r requirements.in + django==1.11.29 \\ + --hash=sha256:014e3392058d94f40569206a24523ce254d55ad2f9f46c6550b0fe2e4f94cf3f \\ + --hash=sha256:4200aefb6678019a0acf0005cd14cfce3a5e6b9b90d06145fcdd2e474ad4329c + # via + # -r requirements.in + # django-debug-toolbar + # django-storages + # django-taggit + pytz==2020.4 \\ + --hash=sha256:3e6b7dd2d1e0a59084bcee14a17af60c5c562cdc16d828e8eba2e683d3a7e268 \\ + --hash=sha256:5c55e189b682d420be27c6995ba6edce0c0a77dd67bfbe2ae6607134d5851ffd + # via + # -r requirements.in + # django + sqlparse==0.3.1 \\ + --hash=sha256:022fb9c87b524d1f7862b3037e541f68597a730a8843245c349fc93e1643dc4e \\ + --hash=sha256:e162203737712307dfe78860cc56c8da8a852ab2ee33750e33aeadf38d12c548 + # via + # -r requirements.in + # django-debug-toolbar + """ + ) + + def test_filter_pip_markers(pip_conf, runner): """ Check that pip-compile works with pip environment markers (PEP496) @@ -911,7 +966,9 @@ def test_multiple_input_files_without_output_file(runner): # pip-compile --no-emit-find-links # small-fake-a==0.1 - # via -c constraints.txt, small-fake-with-deps + # via + # -c constraints.txt + # small-fake-with-deps small-fake-with-deps==0.1 # via -r requirements.in Dry-run, so nothing updated. diff --git a/tests/test_writer.py b/tests/test_writer.py index 2811c4eba..4abbdffcb 100644 --- a/tests/test_writer.py +++ b/tests/test_writer.py @@ -53,21 +53,21 @@ def test_format_requirement_annotation_editable(from_editable, writer): assert writer._format_requirement( ireq - ) == "-e git+git://fake.org/x/y.git#egg=y\n " + comment("# via xyz") + ) == "-e git+git://fake.org/x/y.git#egg=y\n" + comment(" # via xyz") def test_format_requirement_annotation(from_line, writer): ireq = from_line("test==1.2") ireq.comes_from = "xyz" - assert writer._format_requirement(ireq) == "test==1.2\n " + comment("# via xyz") + assert writer._format_requirement(ireq) == "test==1.2\n" + comment(" # via xyz") def test_format_requirement_annotation_lower_case(from_line, writer): ireq = from_line("Test==1.2") ireq.comes_from = "xyz" - assert writer._format_requirement(ireq) == "test==1.2\n " + comment("# via xyz") + assert writer._format_requirement(ireq) == "test==1.2\n" + comment(" # via xyz") def test_format_requirement_for_primary(from_line, writer): @@ -75,7 +75,7 @@ def test_format_requirement_for_primary(from_line, writer): ireq = from_line("test==1.2") ireq.comes_from = "xyz" - assert writer._format_requirement(ireq) == "test==1.2\n " + comment("# via xyz") + assert writer._format_requirement(ireq) == "test==1.2\n" + comment(" # via xyz") def test_format_requirement_for_primary_lower_case(from_line, writer): @@ -83,7 +83,7 @@ def test_format_requirement_for_primary_lower_case(from_line, writer): ireq = from_line("Test==1.2") ireq.comes_from = "xyz" - assert writer._format_requirement(ireq) == "test==1.2\n " + comment("# via xyz") + assert writer._format_requirement(ireq) == "test==1.2\n" + comment(" # via xyz") def test_format_requirement_environment_marker(from_line, writer):