From 2c796710b3f44f0858291a4754bb07ea3f99be02 Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Sun, 4 Oct 2020 20:58:29 +0300 Subject: [PATCH 01/10] Improve test coverage api.py. --- isort/api.py | 2 +- tests/unit/test_api.py | 51 ++++++++++++++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/isort/api.py b/isort/api.py index 6c5876be4..a8cecb623 100644 --- a/isort/api.py +++ b/isort/api.py @@ -354,7 +354,7 @@ def sort_file( try: # Python 3.8+: use `missing_ok=True` instead of try except. tmp_file.unlink() except FileNotFoundError: - pass + pass # pragma: no cover except ExistingSyntaxErrors: warn(f"{actual_file_path} unable to sort due to existing syntax errors") except IntroducedSyntaxErrors: # pragma: no cover diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 1b3ed3701..baec283a1 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -7,36 +7,63 @@ from isort import api from isort.settings import Config +imperfect_content = "import b\nimport a\n" +fixed_content = "import a\nimport b\n" +fixed_diff = "\n+import a\n import b\n-import a\n" -def test_sort_file(tmpdir) -> None: + +@pytest.fixture +def imperfect(tmpdir) -> None: + imperfect_file = tmpdir.join("test_needs_changes.py") + imperfect_file.write_text(imperfect_content, "utf8") + return imperfect_file + + +def test_sort_file_with_bad_syntax(tmpdir) -> None: tmp_file = tmpdir.join("test_bad_syntax.py") - tmp_file.write_text("""print('mismathing quotes")""", "utf8") + tmp_file.write_text("""print('mismatching quotes")""", "utf8") with pytest.warns(UserWarning): api.sort_file(tmp_file, atomic=True) with pytest.warns(UserWarning): api.sort_file(tmp_file, atomic=True, write_to_stdout=True) - imperfect = tmpdir.join("test_needs_changes.py") - imperfect.write_text("import b\nimport a\n", "utf8") - api.sort_file(imperfect, write_to_stdout=True, show_diff=True) +def test_sort_file(imperfect) -> None: + assert api.sort_file(imperfect) + assert imperfect.read() == fixed_content + + +def test_sort_file_to_stdout(capsys, imperfect) -> None: + assert api.sort_file(imperfect, write_to_stdout=True) + out, _ = capsys.readouterr() + assert out == fixed_content + + +def test_other_ask_to_apply(imperfect) -> None: # First show diff, but ensure change wont get written by asking to apply # and ensuring answer is no. with patch("isort.format.input", MagicMock(return_value="n")): - api.sort_file(imperfect, show_diff=True, ask_to_apply=True) + assert not api.sort_file(imperfect, ask_to_apply=True) + assert imperfect.read() == imperfect_content - # Then run again, but apply the change without asking - api.sort_file(imperfect, show_diff=True) + # Then run again, but apply the change (answer is yes) + with patch("isort.format.input", MagicMock(return_value="y")): + assert api.sort_file(imperfect, ask_to_apply=True) + assert imperfect.read() == fixed_content -def test_check_file(tmpdir) -> None: +def test_check_file_no_changes(capsys, tmpdir) -> None: perfect = tmpdir.join("test_no_changes.py") perfect.write_text("import a\nimport b\n", "utf8") assert api.check_file(perfect, show_diff=True) + out, _ = capsys.readouterr() + assert not out + - imperfect = tmpdir.join("test_needs_changes.py") - imperfect.write_text("import b\nimport a\n", "utf8") +def test_check_file_with_changes(capsys, imperfect) -> None: assert not api.check_file(imperfect, show_diff=True) + out, _ = capsys.readouterr() + assert fixed_diff in out def test_sorted_imports_multiple_configs() -> None: @@ -48,7 +75,7 @@ def test_diff_stream() -> None: output = StringIO() assert api.sort_stream(StringIO("import b\nimport a\n"), output, show_diff=True) output.seek(0) - assert "import a\n import b\n" in output.read() + assert fixed_diff in output.read() def test_sort_code_string_mixed_newlines(): From 97686bc4d9034695407682362af8511823574a0e Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Sun, 4 Oct 2020 20:59:44 +0300 Subject: [PATCH 02/10] Improve test coverage hooks.py --- tests/unit/test_hooks.py | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_hooks.py b/tests/unit/test_hooks.py index 083fbfd48..ddaff0e25 100644 --- a/tests/unit/test_hooks.py +++ b/tests/unit/test_hooks.py @@ -31,17 +31,34 @@ def test_git_hook(src_dir): "HEAD", ] - # Test with incorrectly sorted file returned from git + # Test that non python files aren't processed with patch( - "isort.hooks.get_lines", MagicMock(return_value=[os.path.join(src_dir, "main.py")]) - ) as run_mock: + "isort.hooks.get_lines", + MagicMock(return_value=["README.md", "setup.cfg", "LICDENSE", "mkdocs.yml", "test"]), + ): + with patch("subprocess.run", MagicMock()) as run_mock: + hooks.git_hook(modify=True) + run_mock.assert_not_called() - class FakeProcessResponse(object): - stdout = b"import b\nimport a" + mock_main_py = MagicMock(return_value=[os.path.join(src_dir, "main.py")]) - with patch("subprocess.run", MagicMock(return_value=FakeProcessResponse())) as run_mock: - with patch("isort.api", MagicMock(return_value=False)): + mock_imperfect = MagicMock() + mock_imperfect.return_value.stdout = b"import b\nimport a" + + # Test with incorrectly sorted file returned from git + with patch("isort.hooks.get_lines", mock_main_py): + with patch("subprocess.run", mock_imperfect): + with patch("isort.api.sort_file", MagicMock(return_value=False)) as api_mock: hooks.git_hook(modify=True) + api_mock.assert_called_once() + assert api_mock.call_args.args[0] == mock_main_py.return_value[0] + + # Test with sorted file returned from git and modify=False + with patch("isort.hooks.get_lines", mock_main_py): + with patch("subprocess.run", mock_imperfect): + with patch("isort.api.sort_file", MagicMock(return_value=False)) as api_mock: + hooks.git_hook(modify=False) + api_mock.assert_not_called() # Test with skipped file returned from git with patch( From a9853b95e09c98babed94c95d122ac751de60fbe Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Sun, 4 Oct 2020 22:59:49 +0300 Subject: [PATCH 03/10] Improve test coverage of place.py. More characterisation around forced_separate: - what happens when we use * to match the end of packages - changing order of forced_separate sections (ie. making django.utils come before django.contrib) --- tests/unit/test_isort.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_isort.py b/tests/unit/test_isort.py index 19e81b09c..cb911b5ab 100644 --- a/tests/unit/test_isort.py +++ b/tests/unit/test_isort.py @@ -980,6 +980,7 @@ def test_forced_separate() -> None: "from django.db import models\n" "from django.db.models.fields import FieldDoesNotExist\n" "from django.utils import six\n" + "\n" "from django.utils.deprecation import RenameMethodsBase\n" "from django.utils.encoding import force_str, force_text\n" "from django.utils.http import urlencode\n" @@ -993,7 +994,7 @@ def test_forced_separate() -> None: assert ( isort.code( code=test_input, - forced_separate=["django.contrib"], + forced_separate=["django.utils.*", "django.contrib"], known_third_party=["django"], line_length=120, order_by_type=False, @@ -1003,7 +1004,7 @@ def test_forced_separate() -> None: assert ( isort.code( code=test_input, - forced_separate=["django.contrib"], + forced_separate=["django.utils.*", "django.contrib"], known_third_party=["django"], line_length=120, order_by_type=False, From e94d2e3192ce6f2e92309a0706227932e5ef018c Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Mon, 5 Oct 2020 00:27:06 +0300 Subject: [PATCH 04/10] Improve test coverage of sorting.py and setuptools_commands.py pragma: no cover for these. Setuptools commands would be hard to test and sorting.py has an if that should never be reached. --- isort/setuptools_commands.py | 4 ++-- isort/sorting.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/isort/setuptools_commands.py b/isort/setuptools_commands.py index f67008877..96e41dd0b 100644 --- a/isort/setuptools_commands.py +++ b/isort/setuptools_commands.py @@ -31,13 +31,13 @@ def finalize_options(self) -> None: def distribution_files(self) -> Iterator[str]: """Find distribution packages.""" # This is verbatim from flake8 - if self.distribution.packages: + if self.distribution.packages: # pragma: no cover package_dirs = self.distribution.package_dir or {} for package in self.distribution.packages: pkg_dir = package if package in package_dirs: pkg_dir = package_dirs[package] - elif "" in package_dirs: + elif "" in package_dirs: # pragma: no cover pkg_dir = package_dirs[""] + os.path.sep + pkg_dir yield pkg_dir.replace(".", os.path.sep) diff --git a/isort/sorting.py b/isort/sorting.py index 3d3961367..cab77011b 100644 --- a/isort/sorting.py +++ b/isort/sorting.py @@ -64,7 +64,7 @@ def section_key( if reverse_relative and line.startswith("from ."): match = re.match(r"^from (\.+)\s*(.*)", line) - if match: + if match: # pragma: no cover - regex always matches if line starts with "from ." line = f"from {' '.join(match.groups())}" if group_by_package and line.strip().startswith("from"): line = line.split(" import", 1)[0] From c92b7608c3f7b40e991a70388e128a20eed42eb8 Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Mon, 5 Oct 2020 01:19:55 +0300 Subject: [PATCH 05/10] Fix for test_hooks.py Mock.call_args.args can be used only on Python 3.8, fails on 3.6 --- tests/unit/test_hooks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_hooks.py b/tests/unit/test_hooks.py index ddaff0e25..2757f414f 100644 --- a/tests/unit/test_hooks.py +++ b/tests/unit/test_hooks.py @@ -51,7 +51,7 @@ def test_git_hook(src_dir): with patch("isort.api.sort_file", MagicMock(return_value=False)) as api_mock: hooks.git_hook(modify=True) api_mock.assert_called_once() - assert api_mock.call_args.args[0] == mock_main_py.return_value[0] + assert api_mock.call_args[0][0] == mock_main_py.return_value[0] # Test with sorted file returned from git and modify=False with patch("isort.hooks.get_lines", mock_main_py): From bdd1ebf0b50d8b0e8fc43a624db73966319c8ef6 Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Mon, 5 Oct 2020 01:37:40 +0300 Subject: [PATCH 06/10] Fix test failures on Windows. --- tests/unit/test_api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index baec283a1..e888b24e7 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -1,5 +1,6 @@ """Tests the isort API module""" from io import StringIO +import os from unittest.mock import MagicMock, patch import pytest @@ -8,8 +9,8 @@ from isort.settings import Config imperfect_content = "import b\nimport a\n" -fixed_content = "import a\nimport b\n" -fixed_diff = "\n+import a\n import b\n-import a\n" +fixed_content = "import a\nimport b\n".replace('\n', os.linesep) +fixed_diff = "\n+import a\n import b\n-import a\n".replace ('\n', os.linesep) @pytest.fixture From 7bd1511a7daf5dadc1234faef4cc88c8f7ec61cd Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Mon, 5 Oct 2020 01:42:52 +0300 Subject: [PATCH 07/10] Linting. --- tests/unit/test_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index e888b24e7..920890568 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -1,6 +1,6 @@ """Tests the isort API module""" -from io import StringIO import os +from io import StringIO from unittest.mock import MagicMock, patch import pytest @@ -9,8 +9,8 @@ from isort.settings import Config imperfect_content = "import b\nimport a\n" -fixed_content = "import a\nimport b\n".replace('\n', os.linesep) -fixed_diff = "\n+import a\n import b\n-import a\n".replace ('\n', os.linesep) +fixed_content = "import a\nimport b\n".replace("\n", os.linesep) +fixed_diff = "\n+import a\n import b\n-import a\n".replace("\n", os.linesep) @pytest.fixture From fcd020695c95647d77e57d508fb905d6d0cab13b Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Mon, 5 Oct 2020 02:10:19 +0300 Subject: [PATCH 08/10] Another go at fixing the WIndows tests. --- tests/unit/test_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 920890568..c60b19f8d 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -9,8 +9,8 @@ from isort.settings import Config imperfect_content = "import b\nimport a\n" -fixed_content = "import a\nimport b\n".replace("\n", os.linesep) -fixed_diff = "\n+import a\n import b\n-import a\n".replace("\n", os.linesep) +fixed_content = "import a\nimport b\n" +fixed_diff = "\n+import a\n import b\n-import a\n" @pytest.fixture @@ -37,7 +37,7 @@ def test_sort_file(imperfect) -> None: def test_sort_file_to_stdout(capsys, imperfect) -> None: assert api.sort_file(imperfect, write_to_stdout=True) out, _ = capsys.readouterr() - assert out == fixed_content + assert out == fixed_content.replace("\n", os.linesep) def test_other_ask_to_apply(imperfect) -> None: From fa65b2d2500692d7d3ff97a9805d0ee56b917a50 Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Mon, 5 Oct 2020 02:20:28 +0300 Subject: [PATCH 09/10] Another go at fixing the Windows tests. --- tests/unit/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index c60b19f8d..1235626ed 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -64,7 +64,7 @@ def test_check_file_no_changes(capsys, tmpdir) -> None: def test_check_file_with_changes(capsys, imperfect) -> None: assert not api.check_file(imperfect, show_diff=True) out, _ = capsys.readouterr() - assert fixed_diff in out + assert fixed_diff.replace("\n", os.linesep) in out def test_sorted_imports_multiple_configs() -> None: From 4085dd5f1bc79dce8c2c3116ca1e6f36c15a43e3 Mon Sep 17 00:00:00 2001 From: Tamas Szabo Date: Mon, 5 Oct 2020 02:31:14 +0300 Subject: [PATCH 10/10] And another go to fix the Windows tests. ... it is getting late :-) --- tests/unit/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 1235626ed..3d257e705 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -10,7 +10,7 @@ imperfect_content = "import b\nimport a\n" fixed_content = "import a\nimport b\n" -fixed_diff = "\n+import a\n import b\n-import a\n" +fixed_diff = "+import a\n import b\n-import a\n" @pytest.fixture