From cc692998f589de825ef91f17b74a980340c88c37 Mon Sep 17 00:00:00 2001 From: Chris Wesseling Date: Mon, 8 Nov 2021 22:23:49 +0100 Subject: [PATCH 1/4] bpo-33927: Handle json.tool --json-lines infile infile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Handles the remaining case that bpo-45644 left open of bpo-33927. https://bugs.python.org/issue45644#msg405913 Thanks Rémi Lapeyre for pointing that out. --- Lib/json/tool.py | 4 ++++ Lib/test/test_json/test_tool.py | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/Lib/json/tool.py b/Lib/json/tool.py index 0490b8c0be11df..a0a8233ec61b50 100644 --- a/Lib/json/tool.py +++ b/Lib/json/tool.py @@ -69,6 +69,10 @@ def main(): if options.outfile is None: out = sys.stdout else: + if (options.outfile.exists() and + options.outfile.expanduser().samefile(infile.name)): + # exhaust the generator before opening for writing + objs = list(objs) out = options.outfile.open('w', encoding='utf-8') with out as outfile: for obj in objs: diff --git a/Lib/test/test_json/test_tool.py b/Lib/test/test_json/test_tool.py index 1d7fca6efb1cc7..22f23504f3247e 100644 --- a/Lib/test/test_json/test_tool.py +++ b/Lib/test/test_json/test_tool.py @@ -146,6 +146,40 @@ def test_jsonlines(self): self.assertEqual(process.stdout, self.jsonlines_expect) self.assertEqual(process.stderr, '') + def test_writing_jsonlines_in_place(self): + infile = self._create_infile(data=self.jsonlines_raw) + rc, out, err = assert_python_ok('-m', 'json.tool', '--json-lines', infile, infile) + with open(infile, "r", encoding="utf-8") as fp: + self.assertEqual(fp.read(), self.jsonlines_expect) + self.assertEqual(rc, 0) + self.assertEqual(out, b'') + self.assertEqual(err, b'') + + def test_jsonlines_from_a_stream(self): + stream_code = textwrap.dedent(""" + # print a stream of JSON integers + i = 0 + while True: + print(i) + i += 1 + """) + stream = subprocess.Popen((sys.executable, '-u', '-c', stream_code), + stdout=subprocess.PIPE, + bufsize=0, + ) + args = sys.executable, '-u', '-m', 'json.tool', '--json-lines' + with self.assertRaises(subprocess.TimeoutExpired) as timeout: + subprocess.run(args, stdin=stream.stdout, + capture_output=True, timeout=1, + bufsize=0) + stream.kill() + stream.stdout.read() + stream.stdout.close() + stream.wait() + + self.assertIsNotNone(timeout.exception.stdout) + self.assertTrue(timeout.exception.stdout.startswith(b'0\n1\n2\n')) + def test_help_flag(self): rc, out, err = assert_python_ok('-m', 'json.tool', '-h') self.assertEqual(rc, 0) From ae4bd85b1a752979a5a1865345237ccf12b73788 Mon Sep 17 00:00:00 2001 From: Chris Wesseling Date: Mon, 8 Nov 2021 23:01:08 +0100 Subject: [PATCH 2/4] Properly cleanup stream process. Even if in the expected TimeoutExpired isn't raised. --- Lib/test/support/script_helper.py | 19 ++++++++++++++++--- Lib/test/test_json/test_tool.py | 9 +++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/Lib/test/support/script_helper.py b/Lib/test/support/script_helper.py index 6d699c8486cd20..e1ba75196e0cca 100644 --- a/Lib/test/support/script_helper.py +++ b/Lib/test/support/script_helper.py @@ -2,6 +2,7 @@ # e.g. test_cmd_line, test_cmd_line_script and test_runpy import collections +import contextlib import importlib import sys import os @@ -196,9 +197,11 @@ def spawn_python(*args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, **kw): def kill_python(p): """Run the given Popen process until completion and return stdout.""" - p.stdin.close() - data = p.stdout.read() - p.stdout.close() + if p.stdin: + p.stdin.close() + if p.stdout: + data = p.stdout.read() + p.stdout.close() # try to cleanup the child so we don't appear to leak when running # with regrtest -R. p.wait() @@ -206,6 +209,16 @@ def kill_python(p): return data +@contextlib.contextmanager +def killing(process): + """Context manager that kills and cleans up the given Popen on exit.""" + try: + yield process + finally: + process.kill() + kill_python(process) + + def make_script(script_dir, script_basename, source, omit_suffix=False): script_filename = script_basename if not omit_suffix: diff --git a/Lib/test/test_json/test_tool.py b/Lib/test/test_json/test_tool.py index 22f23504f3247e..0cbd653eb1aef5 100644 --- a/Lib/test/test_json/test_tool.py +++ b/Lib/test/test_json/test_tool.py @@ -7,7 +7,7 @@ from test import support from test.support import os_helper -from test.support.script_helper import assert_python_ok +from test.support.script_helper import assert_python_ok, killing class TestTool(unittest.TestCase): @@ -168,14 +168,11 @@ def test_jsonlines_from_a_stream(self): bufsize=0, ) args = sys.executable, '-u', '-m', 'json.tool', '--json-lines' - with self.assertRaises(subprocess.TimeoutExpired) as timeout: + with killing(stream), \ + self.assertRaises(subprocess.TimeoutExpired) as timeout: subprocess.run(args, stdin=stream.stdout, capture_output=True, timeout=1, bufsize=0) - stream.kill() - stream.stdout.read() - stream.stdout.close() - stream.wait() self.assertIsNotNone(timeout.exception.stdout) self.assertTrue(timeout.exception.stdout.startswith(b'0\n1\n2\n')) From 5643ee2c3848551221b847db7c203939d6dc16ac Mon Sep 17 00:00:00 2001 From: Chris Wesseling Date: Mon, 8 Nov 2021 23:07:03 +0100 Subject: [PATCH 3/4] Fix possible NameError if process has no stdout --- Lib/test/support/script_helper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/support/script_helper.py b/Lib/test/support/script_helper.py index e1ba75196e0cca..10b04950a82ec4 100644 --- a/Lib/test/support/script_helper.py +++ b/Lib/test/support/script_helper.py @@ -197,6 +197,7 @@ def spawn_python(*args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, **kw): def kill_python(p): """Run the given Popen process until completion and return stdout.""" + data = None if p.stdin: p.stdin.close() if p.stdout: From 9288cce01a8ab1c7bfd43c0a7d08ca95b502a9f1 Mon Sep 17 00:00:00 2001 From: Chris Wesseling Date: Mon, 8 Nov 2021 23:54:24 +0100 Subject: [PATCH 4/4] Fix test for Windows line endings. --- Lib/test/test_json/test_tool.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_json/test_tool.py b/Lib/test/test_json/test_tool.py index 0cbd653eb1aef5..c67f9115a93479 100644 --- a/Lib/test/test_json/test_tool.py +++ b/Lib/test/test_json/test_tool.py @@ -175,7 +175,10 @@ def test_jsonlines_from_a_stream(self): bufsize=0) self.assertIsNotNone(timeout.exception.stdout) - self.assertTrue(timeout.exception.stdout.startswith(b'0\n1\n2\n')) + self.assertEqual( + timeout.exception.stdout.splitlines()[:3], + [b'0', b'1', b'2'] + ) def test_help_flag(self): rc, out, err = assert_python_ok('-m', 'json.tool', '-h')