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

test: add extra space in pseudo-tty test failure output by Ayase-252 · Pull Request #37957 · nodejs/node · GitHub

/ node Public

test: add extra space in pseudo-tty test failure output - #37957

Merged
aduh95 merged 1 commit into
nodejs:masterfrom
Ayase-252:fix/improve-test-readability
Apr 3, 2021
ConversationCommits (1)ChecksFiles changed
Merged

test: add extra space in pseudo-tty test failure output#37957
aduh95 merged 1 commit into
nodejs:masterfrom
Ayase-252:fix/improve-test-readability

Conversation

Ayase-252 commented Mar 28, 2021

Copy link
Copy Markdown
Member

It adds a extra space between test suite name and the failure reason when a test in pseduo-tty fails. It imporoves the readablity of failure message.

Considering test case

// test/pesudo-tty/test-of-course-it-will-fail-line.js
'use strict'

process.stdout.write("no, no")

and expected output

of course, it fails intentionally
I have two line

Before the change, the test failure is shown as

Before
[00:00|%   0|+   0|-   0]: release test-of-course-it-will-fail-linelength differs.
expect=2
actual=1
patterns:
pattern = ^of\ course\,\ it\ fails\ intentionally$
pattern = ^I\ have\ two\ line$
outlines:
outline = no, no
=== release test-of-course-it-will-fail-line ===                   
Path: pseudo-tty/test-of-course-it-will-fail-line
no, no
Command: /System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python /Users/qingyudeng/projects/node/test/pseudo-tty/pty_helper.py out/Release/node /Users/qingyudeng/projects/node/test/pseudo-tty/test-of-course-it-will-fail-line.js
[00:00|% 100|+   0|-   1]: Done

After the change, it will be shown as

[00:00|%   0|+   0|-   0]: release test-of-course-it-will-fail-line length differs.
expect=2
actual=1
patterns:
pattern = ^of\ course\,\ it\ fails\ intentionally$
pattern = ^I\ have\ two\ line$
outlines:
outline = no, no
=== release test-of-course-it-will-fail-line ===                   
Path: pseudo-tty/test-of-course-it-will-fail-line
no, no
Command: /System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python /Users/qingyudeng/projects/node/test/pseudo-tty/pty_helper.py out/Release/node /Users/qingyudeng/projects/node/test/pseudo-tty/test-of-course-it-will-fail-line.js
[00:00|% 100|+   0|-   1]: Done

Also, it happens when match failure occurs

Before

[00:00|%   0|+   0|-   0]: release test-of-course-it-will-failmatch failed
line=0
expect=^of\ course\,\ it\ fails\ intentionally$
actual=no, no
=== release test-of-course-it-will-fail ===                   
Path: pseudo-tty/test-of-course-it-will-fail
no, no
Command: /System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python /Users/qingyudeng/projects/node/test/pseudo-tty/pty_helper.py out/Release/node /Users/qingyudeng/projects/node/test/pseudo-tty/test-of-course-it-will-fail.js

After

[00:00|%   0|+   0|-   0]: release test-of-course-it-will-fail match failed
line=0
expect=^of\ course\,\ it\ fails\ intentionally$
actual=no, no
=== release test-of-course-it-will-fail ===                   
Path: pseudo-tty/test-of-course-it-will-fail
no, no
Command: /System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python /Users/qingyudeng/projects/node/test/pseudo-tty/pty_helper.py out/Release/node /Users/qingyudeng/projects/node/test/pseudo-tty/test-of-course-it-will-fail.js
[00:00|% 100|+   0|-   1]: Done

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. tty Issues and PRs related to the tty subsystem. labels Mar 28, 2021

nodejs-github-bot commented Apr 3, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/37118/

RaisinTen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Apr 3, 2021
test: add extra space in test failure output
a59b6a1 
PR-URL: nodejs#37957
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
aduh95 force-pushed the fix/improve-test-readability branch from 58d4f49 to a59b6a1 Compare April 3, 2021 18:07

aduh95 commented Apr 3, 2021

Copy link
Copy Markdown
Contributor

Landed in a59b6a1

aduh95 merged commit a59b6a1 into nodejs:master Apr 3, 2021
MylesBorins pushed a commit that referenced this pull request Apr 4, 2021
test: add extra space in test failure output
2a36867 
PR-URL: #37957
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
MylesBorins mentioned this pull request Apr 4, 2021
Merged
Ayase-252 deleted the fix/improve-test-readability branch April 5, 2021 14:33
MylesBorins pushed a commit that referenced this pull request Apr 5, 2021
test: add extra space in test failure output
3175559 
PR-URL: #37957
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
targos pushed a commit that referenced this pull request May 1, 2021
test: add extra space in test failure output
8590720 
PR-URL: #37957
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams mentioned this pull request May 3, 2021
Merged
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Trott Trott approved these changes
lpinca lpinca approved these changes
cjihrig cjihrig approved these changes
richardlau richardlau approved these changes
RaisinTen RaisinTen approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. tty Issues and PRs related to the tty subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL