Repository: timothycrosley/isort
Pull request #809 (2019-01-25 14:54:39)

Test coverage improvements
* The main.py module had been left mostly untested as far as coverage showed, so this PR adds a test to see that the main bits in there don't smoke an awful lot when plugged in.
* There was needless duplication in the multiprocessing code, which is now neatly DRYed out.  
  In addition, use of `concurrent.futures` (3.2+) was replaced with `multiprocessing` (2.6+). (Not that it matters an awful lot with #782 pending, but still, it felt cleaner like this.)


Patch:
diff --git a/isort/main.py b/isort/main.py
index 8cba7b40..a584c226 100644
--- a/isort/main.py
+++ b/isort/main.py
@@ -25,7 +25,6 @@ import glob
 import os
 import re
 import sys
-from concurrent.futures import ProcessPoolExecutor
 
 import setuptools
 
@@ -169,10 +168,10 @@ class ISortCommand(setuptools.Command):
                 except IOError as e:
                     print("WARNING: Unable to parse file {0} due to {1}".format(python_file, e))
         if wrong_sorted_files:
-            exit(1)
+            sys.exit(1)
 
 
-def create_parser():
+def parse_args(argv=None):
     parser = argparse.ArgumentParser(description='Sort Python import definitions alphabetically '
                                                  'within logical sections.')
     inline_args_group = parser.add_mutually_exclusive_group()
@@ -290,7 +289,7 @@ def create_parser():
                              'where it may not be safe to operate in')
     parser.add_argument('files', nargs='*', help='One or more Python source files that need their imports sorted.')
 
-    arguments = {key: value for key, value in vars(parser.parse_args()).items() if value}
+    arguments = {key: value for key, value in vars(parser.parse_args(argv)).items() if value}
     if 'dont_order_by_type' in arguments:
         arguments['order_by_type'] = False
     if arguments.pop('unsafe', False):
@@ -298,8 +297,8 @@ def create_parser():
     return arguments
 
 
-def main():
-    arguments = create_parser()
+def main(argv=None):
+    arguments = parse_args(argv)
     if arguments.get('show_version'):
         print(INTRO)
         return
@@ -334,31 +333,26 @@ def main():
         num_skipped = 0
         if config['verbose'] or config.get('show_logo', False):
             print(INTRO)
+
         jobs = arguments.get('jobs')
         if jobs:
-            executor = ProcessPoolExecutor(max_workers=jobs)
-
-            for sort_attempt in executor.map(functools.partial(sort_imports, **arguments), file_names):
-                if not sort_attempt:
-                    continue
-                incorrectly_sorted = sort_attempt.incorrectly_sorted
-                if arguments.get('check', False) and incorrectly_sorted:
-                    wrong_sorted_files = True
-                if sort_attempt.skipped:
-                    num_skipped += 1
+            import multiprocessing
+            executor = multiprocessing.Pool(jobs)
+            attempt_iterator = executor.imap(functools.partial(sort_imports, **arguments), file_names)
         else:
-            for file_name in file_names:
-                try:
-                    sort_attempt = SortImports(file_name, **arguments)
-                    incorrectly_sorted = sort_attempt.incorrectly_sorted
-                    if arguments.get('check', False) and incorrectly_sorted:
-                        wrong_sorted_files = True
-                    if sort_attempt.skipped:
-                        num_skipped += 1
-                except IOError as e:
-                    print("WARNING: Unable to parse file {0} due to {1}".format(file_name, e))
+            attempt_iterator = (sort_imports(file_name, **arguments) for file_name in file_names)
+
+        for sort_attempt in attempt_iterator:
+            if not sort_attempt:
+                continue
+            incorrectly_sorted = sort_attempt.incorrectly_sorted
+            if arguments.get('check', False) and incorrectly_sorted:
+                wrong_sorted_files = True
+            if sort_attempt.skipped:
+                num_skipped += 1
+
         if wrong_sorted_files:
-            exit(1)
+            sys.exit(1)
 
         num_skipped += len(skipped)
         if num_skipped and not arguments.get('quiet', False):


Test patch:
diff --git a/test_isort.py b/test_isort.py
index 732812a6..4fa9928a 100644
--- a/test_isort.py
+++ b/test_isort.py
@@ -2511,6 +2511,8 @@ def test_new_lines_are_preserved():
     assert n_newline_contents == 'import os\nimport sys\n'
 
 
+@pytest.mark.skipif(not finders.RequirementsFinder.enabled, reason='RequirementsFinder not enabled (too old version of pip?)')
+@pytest.mark.skipif(not finders.pipreqs, reason='pipreqs is missing')
 def test_requirements_finder(tmpdir):
     subdir = tmpdir.mkdir('subdir').join("lol.txt")
     subdir.write("flask")
@@ -2562,6 +2564,8 @@ deal = {editable = true, git = "https://github.com/orsinium/deal.git"}
 """
 
 
+@pytest.mark.skipif(not finders.PipfileFinder.enabled, reason='PipfileFinder not enabled (missing requirementslib?)')
+@pytest.mark.skipif(not finders.pipreqs, reason='pipreqs is missing')
 def test_pipfile_finder(tmpdir):
     pipfile = tmpdir.join('Pipfile')
     pipfile.write(PIPFILE)
@@ -2595,6 +2599,33 @@ def test_monkey_patched_urllib():
         from urllib import quote  # noqa: F401
 
 
+def test_argument_parsing():
+    from isort.main import parse_args
+    args = parse_args(['-dt', '-t', 'foo', '--skip=bar', 'baz.py'])
+    assert args['order_by_type'] is False
+    assert args['force_to_top'] == ['foo']
+    assert args['skip'] == ['bar']
+    assert args['files'] == ['baz.py']
+
+
+@pytest.mark.parametrize('multiprocess', (False, True))
+def test_command_line(tmpdir, capfd, multiprocess):
+    from isort.main import main
+    tmpdir.join("file1.py").write("import re\nimport os\n\nimport contextlib\n\n\nimport isort")
+    tmpdir.join("file2.py").write("import collections\nimport time\n\nimport abc\n\n\nimport isort")
+    arguments = ["-rc", str(tmpdir)]
+    if multiprocess:
+        arguments.extend(['--jobs', '2'])
+    main(arguments)
+    assert tmpdir.join("file1.py").read() == "import contextlib\nimport os\nimport re\n\nimport isort\n"
+    assert tmpdir.join("file2.py").read() == "import abc\nimport collections\nimport time\n\nimport isort\n"
+    out, err = capfd.readouterr()
+    assert not err
+    # it informs us about fixing the files:
+    assert str(tmpdir.join("file1.py")) in out
+    assert str(tmpdir.join("file2.py")) in out
+
+
 @pytest.mark.parametrize('enabled', (False, True))
 def test_safety_excludes(tmpdir, enabled):
     tmpdir.join("victim.py").write("# ...")