Skip to content

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Jan 29, 2022

I was able to reproduce the failure on a Mac, and this change fixes it.

https://bugs.python.org/issue44031

@@ -293,8 +293,8 @@ def validate_cmd(self, *args, stdout="", stderr="", partial=False):
_, out, err = script_helper.assert_python_ok('-m', 'tabnanny', *args)
# Note: The `splitlines()` will solve the problem of CRLF(\r) added
# by OS Windows.
out = out.decode('ascii')
err = err.decode('ascii')
out = out.decode('utf-8')
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to use os.fsdecode() instead. subprocess uses the locale encoding, it can be different than UTF-8. Moreover, fsdecode() uses the surrogateescape error handler for undecodable bytes.

@vstinner
Copy link
Member

I confirm that this PR fix https://bugs.python.org/issue44031

Without this PR, the test fails if the Python directory contains Загрузки:

$ pwd
/home/vstinner/python/Загрузки

$ ./python -m test -v test_tabnanny
(...)
ERROR: test_command_usage (test.test_tabnanny.TestCommandLine)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vstinner/python/Загрузки/Lib/test/test_tabnanny.py", line 325, in test_command_usage
    self.validate_cmd(stderr=stderr)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/vstinner/python/Загрузки/Lib/test/test_tabnanny.py", line 297, in validate_cmd
    err = err.decode('ascii')
          ^^^^^^^^^^^^^^^^^^^
UnicodeDecodeError: 'ascii' codec can't decode byte 0xd0 in position 29: ordinal not in range(128)
(...)

With this PR, it works as expected:

$ ./python -m test -v test_tabnanny
(...)
Tests result: SUCCESS

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

@iritkatriel
Copy link
Member Author

@raghavthind2005 FYI - there is a discussion on python-dev about spam activity on GitHub (which wastes our time) and your recent activity on the cpython repo was mentioned there.

See this link: https://mail.python.org/archives/list/[email protected]/message/UO6ZSNWLLXWU7AZ7NGQDTOQ2WVX2ZAZN/

@iritkatriel iritkatriel merged commit 108e66b into python:main Feb 1, 2022
@miss-islington
Copy link
Contributor

Thanks @iritkatriel for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@iritkatriel iritkatriel deleted the bpo-44031-test_tabnanny branch February 1, 2022 10:32
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 1, 2022
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Feb 1, 2022
@bedevere-bot
Copy link

GH-31047 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 1, 2022
@bedevere-bot
Copy link

GH-31048 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Feb 1, 2022
miss-islington added a commit that referenced this pull request Feb 1, 2022
(cherry picked from commit 108e66b)

Co-authored-by: Irit Katriel <[email protected]>
miss-islington added a commit that referenced this pull request Feb 1, 2022
GH-31048)

(cherry picked from commit 108e66b)


Co-authored-by: Irit Katriel <[email protected]>

Automerge-Triggered-By: GH:iritkatriel
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
…-31014) (pythonGH-31048)

(cherry picked from commit 108e66b)


Co-authored-by: Irit Katriel <[email protected]>

Automerge-Triggered-By: GH:iritkatriel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants