FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-39828: Fix json.tool to catch BrokenPipeError. by corona10 · Pull Request #18779 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

bpo-39828: Fix json.tool to catch BrokenPipeError. #18779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vstinner merged 8 commits into from
Mar 10, 2020
Conversation 37 Commits 8 Checks 0 Files changed
Merged

bpo-39828: Fix json.tool to catch BrokenPipeError. #18779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f1fc4ab
bpo-39828: Fix json.tool to ignore BrokenPipeError.
corona10 Mar 4, 2020
dfa361d
bpo-39828: Update
corona10 Mar 5, 2020
871eabf
bpo-39828: Update
corona10 Mar 5, 2020
337fdd2
bpo-39828: Apply Victor's code review
corona10 Mar 7, 2020
d02c089
bpo-39828: Update NEWS.d
corona10 Mar 7, 2020
f67630c
bpo-39828: Add unittest
corona10 Mar 8, 2020
e6931d8
bpo-39828: Apply Victor's review
corona10 Mar 10, 2020
e9c7eaa
bpo-39828: Ignore test for Windows
corona10 Mar 10, 2020
File filter

Filter by extension

Filter by extension .py  (2) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
5 changes: 4 additions & 1 deletion Lib/json/tool.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,7 @@ def main():


if __name__ == '__main__':
main()
try:
main()
except BrokenPipeError as exc:
sys.exit(exc.errno)
13 changes: 13 additions & 0 deletions Lib/test/test_json/test_tool.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import errno
import os
import sys
import textwrap
import unittest
import subprocess

from test import support
from test.support.script_helper import assert_python_ok

Expand Down Expand Up @@ -206,3 +208,14 @@ def test_ensure_ascii_default(self):
# asserting an ascii encoded output file
expected = [b'{', rb' "key": "\ud83d\udca9"', b"}"]
self.assertEqual(lines, expected)

@unittest.skipIf(sys.platform =="win32", "The test is failed with ValueError on Windows")

vstinner Mar 9, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Where does the ValueError come from?

corona10 Mar 10, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

https://github.com/python/cpython/pull/18779/checks?check_run_id=496883234

 test_broken_pipe_error (test.test_json.test_tool.TestTool) ... ERROR
  File "D:\a\cpython\cpython\lib\threading.py", line 944, in _bootstrap_inner
    self.run()
  File "D:\a\cpython\cpython\lib\threading.py", line 882, in run
    self._target(*self._args, **self._kwargs)
  File "D:\a\cpython\cpython\lib\subprocess.py", line 1493, in _readerthread
    buffer.append(fh.read())
ValueError: read of closed file
D:\a\cpython\cpython\lib\subprocess.py:1066: ResourceWarning: subprocess 6532 is still running
  _warn("subprocess %s is still running" % self.pid,
ResourceWarning: Enable tracemalloc to get the object allocation traceback
OSError: [Errno 22] Invalid argument

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "D:\a\cpython\cpython\lib\runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "D:\a\cpython\cpython\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "D:\a\cpython\cpython\lib\json\tool.py", line 76, in <module>
    main()
  File "D:\a\cpython\cpython\lib\json\tool.py", line 71, in main
    raise SystemExit(e)

bl-ue May 16, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The test is failed with ValueError on Windows isn't grammatically correct. It should be The test failed with ValueError on Windows (minus the is) 🤔

vstinner May 25, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This PR is now closed. You can write a new PR to enhance the skip message.
def test_broken_pipe_error(self):
cmd = [sys.executable, '-m', 'json.tool']
proc = subprocess.Popen(cmd,
stdout=subprocess.PIPE,
stdin=subprocess.PIPE)
Comment thread
corona10 marked this conversation as resolved.
# bpo-39828: Closing before json.tool attempts to write into stdout.
proc.stdout.close()
proc.communicate(b'"{}"')
self.assertEqual(proc.returncode, errno.EPIPE)
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2020-03-05-00-57-49.bpo-39828.yWq9NJ.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix :mod:`json.tool` to catch :exc:`BrokenPipeError`. Patch by Dong-hee Na.
Back | FazBrowse Home | New Git URL