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

Jump to bottom

Test.pm6 should redirect diagnostics to stdout instead of stderr #6400

Closed
p6rt opened this issue Jul 19, 2017 · 7 comments
Closed

Test.pm6 should redirect diagnostics to stdout instead of stderr #6400

p6rt opened this issue Jul 19, 2017 · 7 comments

Comments

@p6rt
Copy link

p6rt commented Jul 19, 2017

Migrated from rt.perl.org#131767 (status was 'resolved')

Searchable as RT131767$
@p6rt
Copy link
Author

p6rt commented Jul 19, 2017

From @szabgab

We have a test marked todo in Bailador. prove6 displays the failure as follows​:

$ prove6 -l t/30-examples-app.t
t/30-examples-app.t ..1/15
  # Failed test 'route GET /hello/Foo.html'
  # at t/30-examples-app.t line 68
  # expected​: $[200, [​:Content-Type("text/html")], "Hello Foo.html!"]
  # got​: $[404, [​:Content-Type("text/html;charset=UTF-8")],
"<html>\n <head>\n <title>Custom 404 page</title>\n
<meta charset=\"UTF-8\">\n </head>\n <body>\n <h1>Hello,
this is 404 for you.</h1>\n </body>\n</html>\n"]
  # Looks like you failed 1 test of 2
t/30-examples-app.t .. ok
All tests successful.
Files=1, Tests=15, 4 wallclock secs
Result​: PASS

I think this makes people, especially people new to perl* uneasy as
the failure takes up a lot of the reporting and the final success is
less obvious.
I just checked and prove of Perl 5 does the same, but I wonder if it
wouldn't be more user-friendly if the default was to hide the error
from todo tests and only show them in verbose mode.

In addition IMHO the final report should also say "1 todo"

Our repo is https://github.com/Bailador/Bailador

I've posted this to the TAP​::Harness project but Leon Timmermans
directed me to Test.pm6
Raku/tap-harness6#17

"That output has nothing to do with todos, and everything with
diagnostics. By default all output to stderr is untouched by prove6,
and this is the most sensible thing it can do. The real issue is in
Test.pm6, which should redirect diagnostics to stdout instead of
stderr when inside of a subtest (this is what Test​::Builder does on
perl5)."

@p6rt
Copy link
Author

p6rt commented Jul 19, 2017

From @zoffixznet

On Wed, 19 Jul 2017 01​:13​:30 -0700, szabgab@​gmail.com wrote​:

We have a test marked todo in Bailador. prove6 displays the failure as
follows​:

$ prove6 -l t/30-examples-app.t
t/30-examples-app.t ..1/15
# Failed test 'route GET /hello/Foo.html'
# at t/30-examples-app.t line 68
# expected​: $[200, [​:Content-Type("text/html")], "Hello
Foo.html!"]
# got​: $[404, [​:Content-Type("text/html;charset=UTF-8")],
"<html>\n <head>\n <title>Custom 404 page</title>\n
<meta charset=\"UTF-8\">\n </head>\n <body>\n <h1>Hello,
this is 404 for you.</h1>\n </body>\n</html>\n"]
# Looks like you failed 1 test of 2
t/30-examples-app.t .. ok
All tests successful.
Files=1, Tests=15, 4 wallclock secs
Result​: PASS

I think this makes people, especially people new to perl* uneasy as
the failure takes up a lot of the reporting and the final success is
less obvious.
I just checked and prove of Perl 5 does the same, but I wonder if it
wouldn't be more user-friendly if the default was to hide the error
from todo tests and only show them in verbose mode.

In addition IMHO the final report should also say "1 todo"

Our repo is https://github.com/Bailador/Bailador

I've posted this to the TAP​::Harness project but Leon Timmermans
directed me to Test.pm6
Raku/tap-harness6#17

"That output has nothing to do with todos, and everything with
diagnostics. By default all output to stderr is untouched by prove6,
and this is the most sensible thing it can do. The real issue is in
Test.pm6, which should redirect diagnostics to stdout instead of
stderr when inside of a subtest (this is what Test​::Builder does on
perl5)."

Gonna take care of this in the next 26hr.... What happens is the tests inside the subtest currently have no
idea its todoed, so they're reported as legit failures instead of todos.

I think there was another ticket for this issue but I'm failing to find it.

Cheers,
ZZ

@p6rt
Copy link
Author

p6rt commented Jul 19, 2017

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Jul 20, 2017

From @zoffixznet

On Wed, 19 Jul 2017 16​:46​:20 -0700, cpan@​zoffix.com wrote​:

Gonna take care of this in the next 26hr....

I lied. Need more time.

The fix I planned to do solved the issue, but now it's reporting the subtest as an unexpectedly-passing TODO heh :P

Gonna think about it for another day or two, given it's a spaghettified and very performance-sensitive part of the codebase...

@p6rt
Copy link
Author

p6rt commented Jul 21, 2017

From @zoffixznet

On Wed, 19 Jul 2017 01​:13​:30 -0700, szabgab@​gmail.com wrote​:

We have a test marked todo in Bailador. prove6 displays the failure as
follows​:

$ prove6 -l t/30-examples-app.t
t/30-examples-app.t ..1/15
# Failed test 'route GET /hello/Foo.html'
# at t/30-examples-app.t line 68
# expected​: $[200, [​:Content-Type("text/html")], "Hello
Foo.html!"]
# got​: $[404, [​:Content-Type("text/html;charset=UTF-8")],
"<html>\n <head>\n <title>Custom 404 page</title>\n
<meta charset=\"UTF-8\">\n </head>\n <body>\n <h1>Hello,
this is 404 for you.</h1>\n </body>\n</html>\n"]
# Looks like you failed 1 test of 2
t/30-examples-app.t .. ok
All tests successful.
Files=1, Tests=15, 4 wallclock secs
Result​: PASS

I think this makes people, especially people new to perl* uneasy as
the failure takes up a lot of the reporting and the final success is
less obvious.
I just checked and prove of Perl 5 does the same, but I wonder if it
wouldn't be more user-friendly if the default was to hide the error
from todo tests and only show them in verbose mode.

In addition IMHO the final report should also say "1 todo"

Our repo is https://github.com/Bailador/Bailador

I've posted this to the TAP​::Harness project but Leon Timmermans
directed me to Test.pm6
Raku/tap-harness6#17

"That output has nothing to do with todos, and everything with
diagnostics. By default all output to stderr is untouched by prove6,
and this is the most sensible thing it can do. The real issue is in
Test.pm6, which should redirect diagnostics to stdout instead of
stderr when inside of a subtest (this is what Test​::Builder does on
perl5)."

Thank you for the report. This is now fixed.

Fix​: rakudo/rakudo@5b77a8aae1bf41b
Test​: Raku/roast@31c4ad533e5ea2cf3

@p6rt
Copy link
Author

p6rt commented Jul 21, 2017

@zoffixznet - Status changed from 'open' to 'resolved'

@p6rt p6rt closed this as completed Jul 21, 2017
@p6rt
Copy link
Author

p6rt commented Jul 21, 2017

From @szabgab

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@p6rt