classification
Title: Tools/parser/unparse.py does not handle u"bar" correctly
Type: enhancement Stage: resolved
Components: Demos and Tools Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: gvanrossum, levkivskyi, miss-islington, pablogsal, yan12125
Priority: normal Keywords: easy, patch

Created on 2019-05-26 12:41 by yan12125, last changed 2019-05-26 17:10 by pablogsal. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 13583 merged yan12125, 2019-05-26 16:46
Messages (6)
msg343546 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2019-05-26 12:41
Currently (135c6a56e55d2f4f8718b3b9f03ce3c692b15f0f) the following test fails:

./python -m test.regrtest -v test_tools.test_unparse -u cpu

(Note that -u cpu is needed to force test_unparse.DirectoryTestCase to check all files under Lib/ and Lib/test/.)

An example error message:

======================================================================
FAIL: test_files (test.test_tools.test_unparse.DirectoryTestCase) (filename='/home/yen/Projects/cpython/Lib/test/test_typing.py'
)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/yen/Projects/cpython/Lib/test/test_tools/test_unparse.py", line 309, in test_files
    self.check_roundtrip(source)
  File "/home/yen/Projects/cpython/Lib/test/test_tools/test_unparse.py", line 132, in check_roundtrip
    self.assertASTEqual(ast1, ast2)
  File "/home/yen/Projects/cpython/Lib/test/test_tools/test_unparse.py", line 124, in assertASTEqual
    self.assertEqual(ast.dump(ast1), ast.dump(ast2))
AssertionError: 'Modu[88178 chars]kind=\'u\')], ctx=Load())), ctx=Load()))], dec[421987 chars]=[])' != 'Modu[88178 chars]kind=No
ne)], ctx=Load())), ctx=Load()))], deco[421986 chars]=[])'

----------------------------------------------------------------------

Apparently that's because Tools/parser/unparse.py does not handle strings like u"bar" correctly. I managed to "fix" it with the following patch:

diff --git a/Tools/parser/unparse.py b/Tools/parser/unparse.py
index 385902ef4b..a25b5e49df 100644
--- a/Tools/parser/unparse.py
+++ b/Tools/parser/unparse.py
@@ -399,6 +399,8 @@ class Unparser:
         elif value is ...:
             self.write("...")
         else:
+            if t.kind == 'u':
+                self.write("u")
             self._write_constant(t.value)

     def _List(self, t):

Not sure if this is the correct approach, though.
msg343554 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2019-05-26 15:16
On the other hand, if this causes troubles, feel free to just replace u"bar" with "bar" in that test. It is not really important.
msg343560 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2019-05-26 16:32
Please submit a PR.
msg343566 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2019-05-26 16:56
After reading Python/ast.c, I think my patch is correct, so I submitted it to GitHub as https://github.com/python/cpython/pull/13583 :)
msg343567 - (view) Author: miss-islington (miss-islington) Date: 2019-05-26 17:08
New changeset aaf47caf35984e614d93bd8bea5227df55e0e3e6 by Miss Islington (bot) (Chih-Hsuan Yen) in branch 'master':
bpo-37053: handle strings like u"bar" correctly in Tools/parser/unparse.py (GH-13583)
https://github.com/python/cpython/commit/aaf47caf35984e614d93bd8bea5227df55e0e3e6
msg343568 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-05-26 17:10
Confirmed that this fixes the broken buildbots:

./python -m test test_tools -vvvv -uall -m test_files
== CPython 3.8.0a4+ (heads/master:91f4380ced, May 26 2019, 17:30:21) [GCC 8.3.0]
== Linux-5.0.15-1-MANJARO-x86_64-with-glibc2.29 little-endian
== cwd: /home/pablogsal/github/cpython/build/test_python_25649
== CPU count: 12
== encodings: locale=UTF-8, FS=utf-8
Run tests sequentially
0:00:00 load avg: 0.50 [1/1] test_tools
test_files (test.test_tools.test_unparse.DirectoryTestCase) ... Testing /home/pablogsal/github/cpython/Lib/typing.py
Testing /home/pablogsal/github/cpython/Lib/test/test_typing.py
ok

Ran 1 test in 0.586s

OK

== Tests result: SUCCESS ==

1 test OK.

Total duration: 671 ms
Tests result: SUCCESS

The manual check is needed because the CI in GitHub does not run test_tools over all files so it does not take forever.
History
Date User Action Args
2019-05-26 17:10:39pablogsalsetstatus: open -> closed

nosy: + pablogsal
messages: + msg343568

resolution: fixed
stage: patch review -> resolved
2019-05-26 17:08:23miss-islingtonsetnosy: + miss-islington
messages: + msg343567
2019-05-26 16:56:08yan12125setmessages: + msg343566
2019-05-26 16:46:29yan12125setkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request13490
2019-05-26 16:32:46gvanrossumsetmessages: + msg343560
2019-05-26 15:16:42levkivskyisetnosy: + levkivskyi
messages: + msg343554
2019-05-26 13:12:03serhiy.storchakasetkeywords: + easy
nosy: + gvanrossum

type: behavior -> enhancement
stage: needs patch
2019-05-26 12:41:33yan12125create