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

like tests throwing warnings #335

Closed
steffenw opened this issue Aug 17, 2012 · 5 comments
Closed

like tests throwing warnings #335

steffenw opened this issue Aug 17, 2012 · 5 comments

Comments

@steffenw
Copy link

use strict;
use warnings;

use Test::More tests => 1;

like
    undef,
    qr{abc}xms,
    'should fail without warnings';

__END__

> prove test.t
test.t .. Use of uninitialized value $this in pattern match (m//) at x:\test.t line 6.

#   Failed test 'should fail without warnings'
#   test.t line 6.
#                   undef
#     doesn't match '(?^msx:abc)'
# Looks like you failed 1 test of 1.
test.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests

Test Summary Report
-------------------
test.t (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=1, Tests=1,  0 wallclock secs ( 0.08 usr +  0.00 sys =  0.08 CPU)
Result: FAIL

------------------------------------------------------------------------------------------------------------------------------------------

see Test-Simple-0.98/lib/Test/Builder.pm line 1407

--- original code ---
$test = eval $context . q{$test = $this =~ /$usable_regex/ ? 1 : 0};

--- change to ---
no warnings qw(uninitialized);
$test = eval $context . q{$test = $this =~ /$usable_regex/ };

You can remove that ? 1 : 0 stuff too because ok expects a boolean and not a numeric.
$test contains already a boolean dual value.

--Steffen

@schwern
Copy link
Contributor

schwern commented Aug 18, 2012

Thanks for reporting, and double for including a test and digging into the code!

Turns out this is a quite deliberate feature added back in 2005. See 8b2f0c6. The idea was to make like and cmp_ok act as much like a normal regex and comparison operation thus you can most reliably translate $foo =~ /bar/ into like $foo, qr/bar/ and $this == $that into cmp_ok $this, '==', $that and know the test code hasn't introduced a change.

Now that you've brought it up, I'm not really sure how useful it is because the failure diagnostics clearly tell you that you passed in undef. The original bug request was to do with only cmp_ok which has many more possible warnings than like.

May I ask, what was your test actually doing when you encountered this? Did it expect to pass undef to like?

@steffenw
Copy link
Author

The problem is only the output that confuses. Searching first in my code where the warning is from. See later that the test modules warns. At least I see that I got an undef in like, that is the problem. I don't know a reason to get ok with undef.

@schwern
Copy link
Contributor

schwern commented Aug 18, 2012

Could you tell me what confused you about the warning?

@steffenw
Copy link
Author

The warning itself, because it exists. And the reason was, I have that warning not expected from Test::More. Thourght that was my own fault fist. That's the reason I wrote confused.

@schwern
Copy link
Contributor

schwern commented Apr 23, 2013

I've thought about this some more. I'd remove the warning if there was a legit use for deliberately passing undef to like, since the warning would be unnecessary, and I can't think of one. undef is never going to match.

OTOH the diagnostics already clearly tell you it's undef, so there's not much point to the warning...

So yeah, I'll suppress the uninit warning.

schwern added a commit that referenced this issue Apr 23, 2013
Not much sense, they can see it in the diagnostics, and the warning
message is a bit confusing.

    Use of uninitialized value $this in pattern match (m//) at x:\test.t line 6.

It reports the right location, but there's no $this from the user's
perspective.

This came in because like used to supress all warnings.  That was
fixed in 8b2f0c6 but it let
uninit warnings slip in.

For #335.
@schwern schwern closed this as completed Apr 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants