From 0ff23d4e9fcaf66565206aa2a30cfdea53477f9d Mon Sep 17 00:00:00 2001
From: Joey Pinhas <joey.pinhas@datadoghq.com>
Date: Fri, 13 Sep 2019 14:30:52 -0400
Subject: [PATCH] Remove extra cli flag, and update test case

This commit uses capsys to test the output of the diff, which is now
hidden behind the autofix flag if it's disabled
---
 pre_commit_hooks/pretty_format_json.py |  25 ++----
 tests/pretty_format_json_test.py       | 114 ++++++++++---------------
 2 files changed, 52 insertions(+), 87 deletions(-)

diff --git a/pre_commit_hooks/pretty_format_json.py b/pre_commit_hooks/pretty_format_json.py
index 2e04064..3626332 100644
--- a/pre_commit_hooks/pretty_format_json.py
+++ b/pre_commit_hooks/pretty_format_json.py
@@ -56,12 +56,12 @@ def parse_topkeys(s):  # type: (str) -> List[str]
     return s.split(',')
 
 
-def get_diff(source, target):  # type: (List[str], List[str]) -> str
-    source_lines = ''.join(source).split('\n')
-    target_lines = ''.join(target).split('\n')
-    d = difflib.Differ()
-    diff = d.compare(source_lines, target_lines)
-    return '\n'.join(diff)
+def get_diff(source, target):  # type: (str, str) -> str
+    source_lines = source.splitlines(True)
+    target_lines = target.splitlines(True)
+    diff = ''.join(difflib.ndiff(source_lines, target_lines))
+    print(diff)
+    return diff
 
 
 def main(argv=None):  # type: (Optional[Sequence[str]]) -> int
@@ -105,14 +105,6 @@ def main(argv=None):  # type: (Optional[Sequence[str]]) -> int
         default=[],
         help='Ordered list of keys to keep at the top of JSON hashes',
     )
-    parser.add_argument(
-        '--show-expected',
-        action='store_true',
-        dest='show_expected',
-        default=False,
-        help='Show a diff between the input file and expected (pretty) output',
-    )
-
     parser.add_argument('filenames', nargs='*', help='Filenames to fix')
     args = parser.parse_args(argv)
 
@@ -131,11 +123,10 @@ def main(argv=None):  # type: (Optional[Sequence[str]]) -> int
             if contents != pretty_contents:
                 print('File {} is not pretty-formatted'.format(json_file))
 
-                if args.show_expected:
-                    print(get_diff(contents, list(pretty_contents)))
-
                 if args.autofix:
                     _autofix(json_file, pretty_contents)
+                else:
+                    print(get_diff(''.join(contents), pretty_contents))
 
                 status = 1
         except ValueError:
diff --git a/tests/pretty_format_json_test.py b/tests/pretty_format_json_test.py
index bd84431..3aca165 100644
--- a/tests/pretty_format_json_test.py
+++ b/tests/pretty_format_json_test.py
@@ -3,7 +3,6 @@ import shutil
 import pytest
 from six import PY2
 
-from pre_commit_hooks.pretty_format_json import get_diff
 from pre_commit_hooks.pretty_format_json import main
 from pre_commit_hooks.pretty_format_json import parse_num_to_int
 from testing.util import get_resource_path
@@ -108,76 +107,51 @@ def test_badfile_main():
     assert ret == 1
 
 
-def test_diffing_output():
-    table_tests = [
-        {
-            'name': 'diff_test_1',
-            'source': """
-{
-    "key1": "val1",
-    "key2":   2,
-"array_key": [1, 2, 3],
-        "object":{
-        "bool_key": true
-      }
-}
-""",
-            'target': """
-{
-  "array_key": [
-    1,
-    2,
-    3
-  ],
-  "key1": "val1",
-  "key2": 2,
-  "object": {
-    "bool_key": true
-  }
-}
-""",
-            'expected': """
-{
-+   "array_key": [
-+     1,
+def test_diffing_output(capsys):
+    resource_path = get_resource_path('not_pretty_formatted_json.json')
+    expected_retval = 1
+    expected_diff = '''
+  {
+-     "foo":
+-     "bar",
+-         "alist": [2, 34, 234],
++   "alist": [
 +     2,
-+     3
++     34,
++     234
 +   ],
--     "key1": "val1",
-? --
-
-+   "key1": "val1",
--     "key2":   2,
-? --          --
-
-+   "key2": 2,
-- "array_key": [1, 2, 3],
--         "object":{
-? ------
-
-+   "object": {
-?            +
-
--         "bool_key": true
-? ----
-
-+     "bool_key": true
--       }
-+   }
+-   "blah": null
++   "blah": null,
+?               +
++   "foo": "bar"
   }
-""",
-        },
-        {
-            'name': 'diff_test_2',
-            'source': '',
-            'target': '',
-            'expected': '',
-        },
 
-    ]
-    for test in table_tests:
-        s = list(test['source'])
-        t = list(test['target'])
-        expected = test['expected'].strip()
-        actual = get_diff(s, t).strip()
-        assert actual == expected
+  {
+-     "foo":
+-     "bar",
+-         "alist": [2, 34, 234],
++   "alist": [
++     2,
++     34,
++     234
++   ],
+-   "blah": null
++   "blah": null,
+?               +
++   "foo": "bar"
+  }
+
+
+'''
+    # output should include a line with the filepath, build it here
+    file_output_line = 'File {} is not pretty-formatted'.format(resource_path)
+    # prepend the above line to the diff
+    expected_output = file_output_line + expected_diff
+
+    actual_retval = main([resource_path])
+    actual_output = capsys.readouterr()
+
+    assert actual_retval == expected_retval
+
+    actual_output = '\n'.join(actual_output)
+    assert actual_output == expected_output