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

tools: allow test tap output to include Unicode characters by MoLow · Pull Request #47175 · nodejs/node · GitHub

/ node Public

tools: allow test tap output to include Unicode characters - #47175

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MoLow:allow-tap-to-include-utf
Mar 21, 2023
Merged

tools: allow test tap output to include Unicode characters#47175
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MoLow:allow-tap-to-include-utf

Conversation

MoLow commented Mar 20, 2023

Copy link
Copy Markdown
Member

Fixes #47173

the issue can be easily reproduced locally by adding sys.stdout.reconfigure(encoding='ascii') to the beginning of tools/test.py

nodejs-github-bot added test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Mar 20, 2023

MoLow commented Mar 20, 2023

Copy link
Copy Markdown
Member Author

CC @nodejs/python @richardlau

richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 20, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 20, 2023

Copy link
Copy Markdown
Collaborator

targos added the fast-track PRs that do not need to wait for 48 hours to land. label Mar 20, 2023

Copy link
Copy Markdown
Member

Well this doesn't look quite right 😆:
https://ci.nodejs.org/job/node-test-commit-osx/51087/nodes=osx1015/console

17:24:54 b'ok 2119 parallel/test-readline-async-iterators'
17:24:54   ---
17:24:54   duration_ms: 10.526
17:24:54   ...
17:24:55 b'ok 2120 parallel/test-repl-history-perm'
17:24:55   ---
17:24:55   duration_ms: 0.118
17:24:55   ...

Note the "b" prefix and single quotes. This results in:
https://ci.nodejs.org/job/node-test-commit-osx/51087/nodes=osx1015/

cclauss left a comment
edited
Loading

Copy link
Copy Markdown
Contributor

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

If you want the second representation (b'L\xc3\xbcsai') instead of the first (Lüsai) then I approve.

>>> import logging
>>> logger = logging.getLogger()
>>> logger.warning("Lüsai")
Lüsai
>>> logger.warning("Lüsai".encode("utf-8"))
b'L\xc3\xbcsai'

richardlau left a comment

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

cclauss changed the title tools: allow test tap output to include unicode characters tools: allow test tap output to include Unicode characters Mar 20, 2023
cclauss self-requested a review March 20, 2023 18:13
MoLow force-pushed the allow-tap-to-include-utf branch from 2b47470 to f83d168 Compare March 21, 2023 07:17
MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 21, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 21, 2023

Copy link
Copy Markdown
Collaborator

MoLow requested a review from richardlau March 21, 2023 08:02

Copy link
Copy Markdown
Collaborator

nodejs deleted a comment from github-actions Bot Mar 21, 2023
MoLow added fast-track PRs that do not need to wait for 48 hours to land. and removed fast-track PRs that do not need to wait for 48 hours to land. labels Mar 21, 2023

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @MoLow. Please 👍 to approve.

MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 21, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 21, 2023
nodejs-github-bot merged commit f5eedf4 into nodejs:main Mar 21, 2023

Copy link
Copy Markdown
Collaborator

Landed in f5eedf4

MoLow deleted the allow-tap-to-include-utf branch March 21, 2023 17:28
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
PR-URL: #47175
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
RafaelGSS mentioned this pull request Apr 6, 2023
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
PR-URL: #47175
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
PR-URL: #47175
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Christian Clauss <cclauss@me.com>
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pseudo-tty/test_runner_default_reporter breaks Python based test runner

5 participants


Back | FazBrowse Home | New Git URL