Re: [code-review] Please review String::FlexMatch

On Saturday 06 September 2003 14:52, Marcel Grünauer wrote:
> First of all, I agree with Tony's response. Even Schwern said it:
> 
>     The test should not ignore stringification [...]
>     When writing the test we should not have to be aware that they
>     are really objects with stringification overloaded [...]
>     overload::Method($thing, '""');".
> 
> His position seems clear enough to me.

I respectfully disagree with his position ;-) See other email for many 
reasons.

> Test::Deep for data-driven tests, I have to walk the expected structure
> first and convert it somewhat:
>    __DATA__
>    errors:
>      attr1: A pure string
>      attr2: !perl/deeptest::regex
>        val: '.*/lib/Foo/Bar.pm'
>      attr3: !perl/deeptest::code
>        val: sub { $_[0] eq 'foo' }

You could avoid the conversion step by contructing the Test::Deep::* objects 
directly. re() is just a shortcut wrapper which calls 
Test::Deep::Regex->new() and similarly for all the others. So something like

!perl/Test::Deep::Regex
 val: /blah/

Should do it (I haven't used YAML before but that looks close enough).

By the way, I require // around my regexes, as does Test::More::like(). It 
allows you do add flags like /blah/i. You might want to add that to 
FlexMatch.

> Now $exp can be used with Test::Deep. I've found one (for me) very 
> annoying
> thing in that the expected structure has to be passed as the second 
> argument
> in cmp_deeply and eq_deeply. That is,
> 
>    cmp_deeply($hash, $exp, 'testname')
> 
> will work but
> 
>    cmp_deeply($exp, $hash, 'testname')

That's very deliberate, is_deeply used that order too, although it's only 
relevant in the diag output when it says

got : "wibble"
expected: "wobble"

The first arg is the thing we're testing, the second arg is the template or 
pattern it must match. You cannot allow the tested value to influence the 
test in any way (just like a school won't let a student change a test paper), 
otherwise you risk passing an object that doesn't meet your requirements. 
cmp_deeply is kind of the data structure equivalent of pattern matching with 
strings. Consider

"hello.*" =~ /hello world/

this returns false. The ".*" does nothing because it's in the string that's 
being tested.

I made the point in the other email but I think it's crucial so I'll make it 
again. Consider

$action = what_next($critical_status);
is($action, "shutdown", "reactor will shutdown when critical");

now imagine that what_next() is very complicated and due to a bug somehow ends 
up returning String::FlexMatch->(code => sub {1})

I hope you've got plenty of duct tape and canned food ;-)

> By the way, code() doesn't seem to be documented in Test::Deep. And 
> eq_deeply()
> is said to need a test name as the third argument, when just below in 
> the docs
> it says that it doesn't output diagnostics.

Thanks, I forgot to document it. I threw it in quickly so I could compare 
Test::Deep to your module without having to explain an easily fixed 
difference.

I might actually deprecate eq_deeply() because it's a misleading name, it's 
not really equality at all,

F