Skip to content
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.

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

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

Merged
merged 1 commit into from
Apr 3, 2021

Conversation

Ayase-252
Copy link
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 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
Copy link
Collaborator

@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 3, 2021
PR-URL: nodejs#37957
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@aduh95 aduh95 force-pushed the fix/improve-test-readability branch from 58d4f49 to a59b6a1 Compare April 3, 2021 18:07
@aduh95
Copy link
Contributor

aduh95 commented Apr 3, 2021

Landed in a59b6a1

@aduh95 aduh95 merged commit a59b6a1 into nodejs:master Apr 3, 2021
MylesBorins pushed a commit that referenced this pull request Apr 4, 2021
PR-URL: #37957
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 4, 2021
@Ayase-252 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
PR-URL: #37957
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
PR-URL: #37957
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, 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
Development

Successfully merging this pull request may close these issues.

8 participants