Merge pull request #1528 from PyCQA/improve-test-coverage

Improve test coverage

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

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)
 

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]

tests/unit/test_api.py

@@ -1,4 +1,5 @@
 """Tests the isort API module"""
+import os
 from io import StringIO
 from unittest.mock import MagicMock, patch
 
@@ -7,36 +8,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 = "+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.replace("\n", os.linesep)
+
+
+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.replace("\n", os.linesep) in out
 
 
 def test_sorted_imports_multiple_configs() -> None:
@@ -48,7 +76,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():

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[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):
+        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(

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,