-
Notifications
You must be signed in to change notification settings - Fork 93
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
explain returns the number of arguments in scalar context #1027
Comments
I am reluctant to change this for the following reasons, but I am open to debate over it, I would want other toolchain people to weigh in.
So, usually for changes like this, where it is possible someone depends on Test::More's existing, (despite being demonstrably bad, as you have pointed out) behavior. The answer is "Implement a fix in Test2, so that no Test::More stuff breaks" and if it breaks existing Test2::V0 functionality implement it for a new Test2::V* bundle to avoid breaking things for Test2 consumers. The issue with that response here is that the main Test2 distribution does not include an explain(), largely because I refused to take a side in the debate on how explain should work which caused a schism between the previous Test::More maintainer and the Test::Most author. This schism resulted in a major implementation incompatibility that made creating Test2 significantly harder, thus I chose to protest by not implementing explain() at all and telling people who wanted it to write Test2::Explain and put it on cpan so that they could be responsible for the bike-shedding. |
I figured as much, but a note in the docs would be fine, and have it acknowledge in the docs as a known issue that will not fixed. I'm just going to stop using As I've noted before, Test2 is a non-starter for most of my stuff because it wasn't included with perl until 5.25.1 and my support window is much earlier than that and I'd rather stick to what is in core. As it is now, I'm not migrating anything to Test2, which would effectively be starting over. |
This issue is mostly here to check your appetite for change and so people can see that it's a weird behavior.
explain
doesn't work in scalar context:This outputs:
This comes from
Test::Builder::explain
directly returns the result of amap
:There's no documentation about this behavior, and there are no tests for
explain
in scalar context. I don't think the current scalar context behavior is intended, just overlooked.I think this would be much better in scalar context if it joined each string on a newline (or something) and returned one string. If the user wants one thing,
explain
should give back the most useful one thing.The text was updated successfully, but these errors were encountered: