Comments (63)
It is quite possible for an arbitrary reference to numify to something that happens to match whatever is on the rhs.
from test2-suite.
they can explicitly put the object in quotes to get the stringification
I strongly object to this suggestion. If my input is:
$input = { a => { b => [ ..., $c ] } };
I want to be able to write:
is($input, $complicated_test);
and not
is("$input->{a}{b}[9]", $test1);
is("$input->{a}{z}{w}", $test2);
and so on. It is the test specification that should say what it means.
from test2-suite.
I think the following:
- This is getting too magical
- We already have too many DSL functions
- There is not enough consistency of the DSL functions
I think we need to move to the less is more approach. I think the dsl should have only a handful of subs:
is(
$structure,
st Hash => sub {
my $st = shift;
# or
my $st = st_this;
stc key1 => 'val1';
stc key2 => 'val2';
stc nested => st Array => { sta empty => 1 };
stc non_empty_string => st String => { sta empty => 0 };
stc price => st Number => 12;
sta 'end'; # Or to be more explicit:
sta end => 1; # or stp etc => 1,
},
"name"
);
st - define a structure
st Type Val
st Type \&Builder
to define a Test2::Compare::Type check
stc - structure component;
stc key => 'val' # like 'field'
stc idx => 'val' # for arrays (like item with an index)
stc 'val' # for arrays (like item with no idx)
stc $val # String/number/etc.
to define components inside the structure
stcs - structure values
stcs ...; # Specify several components at once
sta - structure attributes
sta 'attr' - toggle attribute on
sta attr => $val - set attribute to value
This is essentially calling a method on the check when used in a builder to toggle attributes
stp - structure property
stp what => val
This would be like a 'prop' to check properties of the thing you got.
Other subs can be added as shortcuts for some common cases, in fact all current dsl subs can be implemented with these.
This interface still allows for builders (the kind that take subs) while allowing for those that do not (like string() and number()) It is a bit more verbose, but it leads to more consistency, and less need to know the differences between similar things.
Negation of a structure check (all checks are now structure checks, just that string and number are not deep structures) will still enforce an instance of the structure, but negate the contents of the structure.
We cannot negate something that isn't assigned. Schwern wanted !empty
called in a void context, I could see this being achievable with some magic, but I really do not want that kind of magic. I think 'sta empty => $trival' where $trival can be true, false, or undefined (allow empty, disallow empty, don't care) is a but more sane.
stc foo => sub { sta empty => 0 }
to check for a non-empty string. Then for petdance he can write a shortcut to contain that with less verbosity.
This solves the array { ... }
vs string(...)
inconsistency as well as both are:
st Array => \@array;
st Array => sub { ... };
st String => "...";
st String => sub { ... };
consistent! no prototypes, no magic!
Final point to address, petdance implied we would also change isnt(). That needs to be discussed. Currently isn't is a full fledged negation, it passes if any of the checks fail. I have been thinking isnt() is it is currently defined is useless for all but shallow checks isnt(undef, 'not_undef')
, which is frankly better written as is(undef, undef)
from test2-suite.
I think this is a good idea, but deciding what "non-blank" is will need some discussion.
First issue is the !ref
check. That should be removed otherwise string and numeric overloaded objects will be "blank". For example.
use Test::More;
use URI;
my $foo = { name => URI->new("http://example.com") };
ok( !ref($foo->{name}) && length($foo->{name}) > 0 ) || diag $foo->{name};
done_testing;
In general, implicitly excluding objects or references from checks will lead to confusing results and pierce the encapsulation of overloaded objects. It could be argued that non-blessed references should be excluded, or even blessed references which are not string or numeric overloaded, but not simply all references.
I say to keep it simple and to let the caller decide whether they want to exclude references.
from test2-suite.
Next is the term "blank". What does it mean? The code says it's something that has a length. What about " "
? That has a length, but it's arguably "blank".
It would be clearer to call this empty_string
defined as "it stringifies to ""
". Then you can use !empty_string
to check that something will stringify to something other than an empty string. This clears up all sorts of ambiguities.
These match empty_string
:
undef
""
URI->new("")
These do not:
0
[]
" "
URI->new("http://example.com")
from test2-suite.
I agree with @schwern here, if the blank/non-blank were to be a core part of Test2::Suite it would need to look like this to be properly generic, and meet expectations.
That said, I am guessing this does not help you with your use case as much as you would like. However you could then wrap this with a couple other exisitng tools to create your_not_blank() to suppliment it in your own test suite's tools.
from test2-suite.
The !ref
is so that you won't pass when passing [...]
or {...}
or an object, which is part of the problem it was designed to solve. You get a ref back from a sub thinking you're going to get a string, and my is_nonblank()
catches that.
The !empty_string
makes sense, but I'm wary of allowing URI->new("")
passing. Or does it pass only because it string overloads, and another object wouldn't?
from test2-suite.
I'm also thinking that it should be string_empty
, not empty_string
, because there could be string_all_caps
or string_url
or any number of string_whatever
coming down the pipe.
from test2-suite.
Something with string in the name should stringify the argument it is given, so if URI->new("") stringifies to "" then it should pass as an empty string.
from test2-suite.
I think empty_string is a better name than string_empty. The other thingsd you mention may come to exist, but seem to specialized for core.
from test2-suite.
I take it back, I think this is confusing. field name => !empty_string
looks like it would match, say, undef
, or a hashref, or 14
, or anything that is not ''
.
from test2-suite.
@petdance URI->new
is a good example because it masquerades as a string. You can pass it around as a string and never know it's an object. That's part of the point of string overloading. So at minimum the test would have to detect if the ref is a string overloaded object (including fallbacks) and let it through. It's not hard, but it adds a bunch of complexity.
As for whether []
and non-string overloaded objects should be an exception... if you use them as strings they stringify to non-empty strings. They're non-empty strings. So when you say this is confusing...
I think this is confusing.
field name => !empty_string
looks like it would match, say, undef, or a hashref, or anything that is not an ''.
I say "works as designed". It keeps it simple. The check does one thing, it checks if a thing will stringify as an empty string or not. This simple definition avoids having to make a bunch of judgement calls and edge cases and impose a philosophy about objects (ie. treating refs and objects as special things). The check only cares about what it stringifies to.
If you want to also check that it's not a reference, make that explicit. field name => !empty_string && !ref
. I don't know if &&
and ||
are overloaded for checks, but it sure would be cool if they were!
from test2-suite.
&& is not overloaded, but it easily could be using the set checks which let you check that all checks within them match, or that only one does, or that any one does... @schwern can you make a ticket for fleshing out that feature?
from test2-suite.
I think calling it non_empty_string
would be best. It's ugly but when I google "non-empty string" I get a decent number of hits, suggesting it has some precedent in terms of how devs talk about these things. Also see https://metacpan.org/pod/MooseX::Types::Common::String for another usage of the term.
from test2-suite.
Also, I think it's incredibly uncommon to check that something is an empty string, whereas checking for non-emptiness is extremely common. I use that MX NonEmptyStr
type all the time in my code.
from test2-suite.
If you want to also check that it's not a reference, make that explicit.
The whole point is to not have to make it explicit. The point is to have a string that is not a reference.
Let me describe the use case. I know it probably doesn't fit the model you have in mind.
Right now I might have code like this:
my $user = get_user();
isa_ok( $user, 'My::User' );
my $name = $user->name;
ok( !ref($name) ); # If I get a reference or object back, that's an error.
ok( length($name) > 0 ); # It can't be blank
my $accounts = $user->accounts;
ok( ref($accounts) eq 'ARRAY' ); # Have to get an array back
ok( @$accounts > 0 ); # Has to be a nonempty array
my $rank = $user->sales_rank;
ok( !ref($rank) ); # Has to not an object of any kind.
like( $rank, qr/$\d+$/ ); # Has to be an integer
ok( $rank > 0 ); # Has to be greater than 0
These are so common I have wrapped them up into
is_nonblank( $user->name );
is_nonempty_array( $user->accounts );
is_positive_integer( $user->sales_rank );
For me, the promise of Test2 is the ability to do this:
is( get_user(),
object {
obj_type => 'My::User';
field name => nonempty_string;
field accounts => nonempty_array;
field sales_rank => positive_integer;
end;
}
);
from test2-suite.
I know it probably doesn't fit the model you have in mind.
Exactly the problem. Not just with my model, but with the probably half a dozen different ways people think about refs and objects in Perl floating around out there. Which is to say that your use case is totally valid; the question is how to solve it without invalidating the other common use cases.
Adding extra implicit caveats to checks restricts how they can be used. If empty_string
means "its an empty string and it's not a reference" it cannot be used by someone like me who doesn't differentiate between "real" strings and things that act like strings.
This is why I advocate simplicity, meaning few moving parts. A check that does one thing has maximum utility for the maximum number of situations and more specialized and complex/compound checks can be built on it.
I think #75 resolves this problem between flexible simplicity and specific complexity by making multiple checks easy to combine without having to write a plethora of new combined checks. Hopefully we can write your example as...
is( get_user(),
object {
obj_type => 'My::User';
field name => !empty_string;
field accounts => !empty_array;
field sales_rank => positive_integer;
}
);
from test2-suite.
As for non_empty_string
that's just begging for double negatives. We have !
now.
from test2-suite.
the question is how to solve it without invalidating the other common use cases.
Agreed 100%. Components are good.
Hopefully we can write your example as...
is( get_user(),
object {
obj_type => 'My::User';
field name => !empty_string;
field accounts => !empty_array;
field sales_rank => positive_integer;
}
);
This goes back to what I said above: field name => !empty_string
looks like it would match, say, undef
, or a hashref, or 14
, or anything that is not ''
. !empty_array
looks like it would match a hashref, or a 14
, or undef
or "Blah blah"
.
from test2-suite.
@petdance Good one, the problem is much clearer with empty_array
. Yeah, empty_blah
should mean "it (is|behaves like) a blah and it is also an empty blah". The "(is|behaves like)" is still there because of the overloading problem: @{}
(ie. dereferencing an array) can be overloaded.
Ok, here's my summary of the approaches.
Invert the check to populated_blah
.
This fixes is {}, populated_array;
since a hash references is not an array with things in it, but it still leaves is {}, !populated_array;
ambiguous.
Anything that isn't a "real" array/hash/number/string fails.
This breaks encapsulation of overloaded objects, very common for stringification. And with so many things in Perl automatically stringifying it's particularly weird for empty_string
.
Only things which behave like an empty blah pass.
I'd argued that people should say is {}, string + empty_string
or something, but this becomes nonsense when talking about arrays and hashes. is {}, empty_array
should not pass and all the empty_blah
checks should behave similarly.
All references fail.
This breaks encapsulation of overloaded objects.
All non-blessed references fail.
Not all objects are overloaded.
First introspect a reference to see if it will behave like a blah.
Like return FAIL if ref $thing && !overload::Method($thing, q[""])
. This is a bunch of internal complexity and I'm not convinced we even know all the ways people can mess with the behavior of references. There's a better option...
Use the behavior of references.
empty_blah
would try to use it as a blah wrapped in a try block. If it works, great! If it doesn't, it fails. I like this over introspecting the thing to see if it implements overloading because we don't have to worry about all the ways a Perl reference might behave differently. It should be documented as "it behaves like a blah and it is an empty blah" ("behaves" as opposed to "is").
Buuut with references stringifying this brings us back to whether is {}, empty_string
should pass or fail. Consistency and behavior of the code says pass. User expectations say fail.
Use the behavior of references, but string and number checks first look for string/number overloading.
This is just like before with special exceptions for all string and numeric checks. They all check for string and numeric overloading (be sure to take into account fallback!) before considering an object as a string.
Yes, numbers. Did you know references can be used as numbers?
$ perl -wle 'print 0 + {}'
140668399801960
Yup, cmp_ok {}, ">", 0;
will pass. That's just a weird quirk that I've never heard being encountered. The DSL is going to bring out this problem. Users probably won't expect is {}, positive_integer
to pass.
Fortunately checking for overloading is safe and easy, Test::More has been doing it for a long time.
I think this is the best thing we can do. It balances consistency with user expectations.
Thoughts?
from test2-suite.
Oh 💩, another edge case. What do we do about this?
my $obj = bless {}, "Some::Class";
is $obj, empty_hash;
Should a blessed hash reference be treated like a hash?
As much as I'd like to say "no, don't peek inside objects!" I realize a large bulk of the Perl 5 community is against me on this one. So I'll concede that should pass.
If you don't want it to pass, write is $obj, !object + empty_hash
or something.
Whew! Thoughts?
from test2-suite.
My thought is that if the name includes the structure type, 'array', 'hash', 'object', etc. That the check should verify that the thing being checked is of that type, or can pretend to be that type via overloading (or is that type but blessed). This includes the negated ones. so !populated_array still implies that it must be an array.
Negation negates the state of the structure, but not the type. This is consistent with how I have been trying to do all the checks.
populated_array and !populated_array should both fail if the thing they are checking is not an array.
This is why checks have both verify() and delta() verify() checks that the thing is the expected type (array), delta() then returns the differences of the contents.
populated_array should have a verify() that returns true/false based on the type of the thing being checked. It's delta() should be run only when verify() passes (that is baked in already) and return differences between got and want knowing they are at least the same type.
from test2-suite.
If an object is a hash, then populated_hash and !populated_hash should both have their verify step pass, cause the thing they got can be used as a hash.
from test2-suite.
Fact is there is ambiguity, we cannot get rid of ambiguity completely, but what we can do is always treat the ambiguous cases consistently, and that means a check against a structure must verify it got something of that structure, even if it's checks inside the structure have been negated via !.
This should be documented as the "consistent" thing to do, and anything we have already that does nto conform to this should be fixed (I am pretty sure everything conforms)
from test2-suite.
@exodist You said "type" and "is" a lot, not "behaves". This is important. Do you mean "behaves like a hash/array/string/number" or do you mean "is a hash/array/string/number"?
from test2-suite.
if it can sanely be treated like an array (is an array, is tied as an array, overloaded to be an array, etc) then verify() should pass on anything that implies it works on an array, even when negated.
from test2-suite.
to be clear "behaves like an array"
from test2-suite.
Thinking about it, the array and hash checks as they stand will fail on anything that is not reftype(thing) eq 'ARRAY'
they should be fixed to let you use anything that can behave as an array/hash/etc.
from test2-suite.
I note that Ref::Util provides is_plain_hashref
, is_blessed_hashref
, and is_hashref
because this is such a mess. FWIW, I'm strongly in favor of defaulting to not peeking inside blessed structures, on the theory that if I blessed it I meant to treat it as an object, not a data structure.
Also, to go back to overloading a bit ... this is a huge mess with string & number overloading, since all Perl types are capable of acting like strings or numbers, but only in the most useless way possible. To actually check whether something acts like a number in Perl is really painful and requires you to complete violate overloading encapsulation. See http://blog.urth.org/2010/10/16/whats-wrong-with-perl-5s-overloading/ for a blog post I wrote about this years ago, as well as Specio's type check's for these things for more painful details.
from test2-suite.
The main difference here is we are not "guessing" in any way. The check on the right hand side dictates what should be in the left hand side (the got structure). So if you do number(5) on the right hand side it will only pass if the left is == 5
when treated as a number. Similar for string("foo"), passes only when the left hand side is "foo" when treated as a string.
In neither case does it inspect internals, or try to guess if it is a number vs string vs ref, it just treats it as what it expects, and fails if that produces no match, passes if it produces a match.
We must not go down the smartmatch road to hell by trying to guess these things.
Carrying this forward, array { ... } should pass when the left hand side can be treated as an array (ie, using $rhs->[0] or @{$rhs} works) and all the indexes match their checks.
If the check contains the word array it implies that the check must be used against an array, or something that behaves as an array. IF the thing it gets does not behave as an array it should fail, and not get to the nested checks, negated or not.
from test2-suite.
It does occur to me that maybe another way to deal with this is to actually have some explicit support for type checks, so you could write something like method foo => is_string() & !empty_string
from test2-suite.
we cannot and should not attempt to protect against that. if you do is('foo', number(0)) it will pass, because left hand side numberifies to 0. this is not a bug, it is by design.
from test2-suite.
Well, I would want to protect against that in my own code, which is I use type checks liberally!
from test2-suite.
right. and you should. but for is('foo', number(0)) to fail would be bad as perl does in fact treat it as the number 0. if you want to be more strict you need to go to extra effort, just like any other perl code. I am all for adding helpers to make that easier, but it is not a sane default.
from test2-suite.
This problem of cmp_ok( {}, '>', 0 )
passing and of isnt( {}, '' )
passing is exactly why I wrote, for example, is_positive_integer
for my own testing. I understand that "That's the way that Perl works". I understand that there is overloading. For me, the point of testing is to protect against these things that can get by.
If I say "I want to verify that $foo
is a nonblank string", that means that I want to verify that $foo
is a scalar, is not a reference, is not an object and is not ''
.
That's why I started Test::Expressive. That's what I need in my testing.
from test2-suite.
I think nonblank_string, when following the policy I specified would have to pass for references, and numbers. however I think we can also add !a_ref and !a_number, etc, that can be used together to achieve what you want. if you then want to wrap them up in another shorcut you can. but I thinking having the 3 combinable ones is more valuable than the shortcut because you can compose a check. the shortcut is still valuable, but not as a core part of the dsl. I don't think nonblank_string makes sense as a core dsl function. however I have no objections to you writing it in a third party module, at which point you are not bound by the rules I am setting for core dsl subs.
from test2-suite.
Sorry, was on my phone when I wrote that, let me try to make it more sane:
- I think for core DSL subs, the behavior I outlined should be policy.
- I think we should try to write small composable pieces, and steer away from precomposed checks
- Third I welcome the idea of helpers that are essentially precomposed combinations, and I think the manual will need to document how to make them easily. But I don't think the combinations should be in core.
So, I think you should write nonblank_string(), I think it should behave exactly the way you want it to for your needs. I think it should live in a third party module. Ideally it will be a shortcut for composing smaller checks such as: not_ref + D() + match qr/\S+/
. I would recommend the negated form negate the match, but not the 'not_ref' check. NOTE: not_ref is not real, yet....
from test2-suite.
I also realize my policy has 1 inconsistency with reality. I have generally been treating undef as a special case. string("") will not match undef even though undef stringifies to "". I think that treating 'undef' as special should be part of the policy, justified by the fact that most things that stringify undef will warn when warnings are on.
So, policy:
- If the sub name contains a type (array, string, number, hash, object) it should only VERIFY if the thing on the left hand side is-a, or can behave as-a TYPE.
- undef is a special case, even though perl can treat it as a string ('') or number (0) the core tools should not.
<does not exist>
is a special case just like undef, $h->{empty} can be treated as 0 or '', but should not be by our tools.- Core tools should be small composable units, IE they check one thing.
- Minimal precombinations are allowed in core as long as they are demonstrably common (example: FDNE - false or does not exist)
- Negation of a check that includes a type name should not negate the type verification, it should only negate the contents of the thing.
- Pre-composing the checks in third party modules is fine, and they are free to deviate from this policy, but should probably document that they do so.
from test2-suite.
I'm fine with small components in core that make it easier to build third-party add-ons. My explanation was there to explain the sorts of meta-checks that I would want to use the components for using.
I don't see anything in the comments you've posted that I would disagree with.
Off the top of my head, the sorts of components I can see using or having to build include:
- is/isn't defined
- is/isn't blessed
- is numeric (probably a la
looks_like_number
) - is an integer
- is a float
- is positive (>0)
- is/isn't negative (<0, >=0)
- is a string
- is a float
- is empty (hash or array)
I know that things like "is positive" are syntactic sugar because it decomposes to just a numeric comparison to zero, and "is empty" is just a numeric comparison of the number of keys to zero, but they're so very common I think they're worth considering putting in core.
So it sounds like my example above might work out to be something like
is( get_user(),
object {
obj_type => 'My::User';
field name => D + !ref + !blank;
field accounts => D + arrayref + !empty;
field sales_rank => D + !ref + integer + positive;
end;
}
);
Ideally it would be nice to be able to do some composition so I could do:
nonblank_string = D + !ref + !blank;
nonempty_array = D + arrayref + !empty;
positive_integer = D + !ref + integer + positive;
to avoid cutting & pasting things like D + !ref + !blank
over and over again.
from test2-suite.
Some of the checks you said we need we actually have, but you use them in your code samples so you probably knew that :-)
You can already easily create compound checks using the sets: https://metacpan.org/pod/Test2::Tools::Compare#SET-BUILDERS
We just need to add some things:
- an 'xor' equivalent set (IE exactly 1 check in the set passes)
- A way to turn a check (or composite of check) into a sub you run to get a clone of that set with the line numbers and file name updated (checks record the file+line where they are defined, not where they are used, the latter is not possible)
- overloading of '+' '&' and '|'
from test2-suite.
I never even noticed some sets. Some examples in the docs would probably be a big help.
So I read it as being able to do
my $nonblank_string = check_set( D, !ref, !blank );
is( $user,
object {
field name => $nonblank_string;
field city => $nonblank_string;
}
);
from test2-suite.
Yes, except that ref and blank are not checks right now.
from test2-suite.
oh also, the line numbers of that will not be ideal, it will report errors in the set to the line hwere the set was defined, not to the line where you used it.
from test2-suite.
Yes, except that ref and blank are not checks right now.
I know, I'm talking high-level here.
What if I did it as
sub nonblank_string {
check_set( D, !ref, !blank );
}
is( $user,
object {
field name => nonblank_string;
}
);
That's really the more likely use anyway, that users will set up their own meta-checks, like maybe
sub isbn10 {
check_set( D, !ref, match qr/^\d{9}[\dX]$/ );
}
from test2-suite.
That will still have the wrong line numbers, it will report to being inside your sub. I anticipated this in the todo list above, I will write a tool to make it easy. Probably do not have time to write the tool today though.
from test2-suite.
to be clear, here is what I have in mind:
# This will define the nonblank() sub in the current namespace
compose_check nonblank => (D, !ref, !blank);
# Then use it.
is($x, nonblank());
Perhapse also this:
use Ndn::Tools::Compare::Compose(
nonblank => [D, !ref, !blank],
);
so that people can define them at BEGIN time easily
also, I think compose_check should define the sub in void context, but return an anonymous sub in non-void context.
from test2-suite.
So long as I can call compose_check nonblank => (D, !ref, !blank);
in My::Local::Test2::Bundle and have it propagate out to calling *.t files. That's what it looks like you're talking about in Ndn::Tools::Compare::Compose?
I just want to be able to
use My::Local::Test2::Bundle;
is( $x, nonblank() );
from test2-suite.
You will be able to.
from test2-suite.
oops, did not intend to close.
from test2-suite.
I'm cool with the policy list exodist posted, with one major exception.
_Treating default reference numification/stringification as a number/string is useless and error prone._
I'm with @autarch and @petdance on that one now. Any "string" and "number" type checks should reject default reference numificaiton and stringification (ie. 140507464357480
and ARRAY(0x7ff829005668)
) because it's a useless feature. I think this special case is worth the effort and complexity, fortunately encapsulated inside Test2, because it reflects user expectations and testing needs. For every time someone might want is {}, !empty_string
to pass there's easily 100 or 1000 or more where it should not. If we require is $foo, real_string + !empty_string
99 out of 100 times people will forget since 99 out of 100 times is $foo, !empty_string
"works" and the mistake is silent.
So long as we do it consistently I don't think anyone will even notice because it Just Works.
Go ahead and look through your existing tests and mentally count which ones really want ARRAY(0x7ff829005668)
to pass a string test. Or how many of your cmp_ok
tests would "pass" if fed a reference.
Exceptional cases should be exceptional. If someone really wants to test default reference numification and stringification, they can explicitly numify or stringify it.
Or hey, checks are functions. Add an option. is {}, !empty_array( allow_bare_refs => 1 );
from test2-suite.
As for non_empty_string that's just begging for double negatives. We have ! now.
Did this get addressed and I missed it? It sounds like the current plan is:
empty_string
means "a string, and it is empty."
and
!empty_string
means "a string, which is not empty."
Which means that every test is defining the semantics of its own negation, and that these two lines are not equivalent:
not( empty_string->matches( $x ));
( ! empty_string)->matches( $x );
This makes me a lot more unhappy than "double negation." If we have two clearly-defined tests, each of which can be negated through a generic mechanism that means "does not pass this," then it's clear. If every assertion has two meanings, I am sure I will end up pretty confused in short order.
from test2-suite.
@rjbs Does #73 (comment) clear up why the type checks are necessary and why they don't also negate?
Looking at it another way, we're negating the "empty" part, not the "type check" part because that would be nonsense. And yes, this is different from negating the result. We can get away with this because it's a DSL and not intended to be used as code with explicit method calls like you have.
This is one of those things that only becomes a problem when you think about it too much. :)
from test2-suite.
@rjbs To put it another way, we intend !empty_box
to scan as "I want a box that's not empty". The literal interpretation of "I want something which isn't an empty box" is awkward. If you said !empty_box
and I handed you a cat, you wouldn't be happy (neither would the cat because they don't have an empty box to play in). Only a robot would be happy.
Maybe what we need is a more generic empty
check! Then we can write:
is $thing, string + empty;
I'm liking that! The bare reference string/number problem is easier to resolve. We can offer string
(anything that behaves like a string except default ref stringification), real_string
(it's a non-reference scalar), and maybe any_string
(which is basically everything but undef
).
This also avoids a proliferation of empty_blah
functions, at least in the Test2 core. The different empty checks for various types would be internal to Test2 and hidden from the user.
OH! Maybe take a page from array
and hash
!
is $thing, string { empty };
I think that's it! Then there's no ambiguity about what this means.
is $thing, string { !empty };
from test2-suite.
@schwern, I agree with what you say. we can make default ref string/numberising to be an exception.
from test2-suite.
Plus, if someone wants to check stringification of their object, they can explicitly put the object in quotes to get the stringification.
my $obj = SomeObj->new;
is( $obj, empty_string ); # Fails because $obj is a reference.
is( "$obj", empty_string ); # Passes because the user has stringified it explicitly.
from test2-suite.
I think the real problem we have here is the classic no-one understands what overloaded negation actually means - at least not consistently with what other people think. This is why I think the magical aspect of it is just a really bad idea and we should avoid it entirely. No matter what we decide it means half the people will think it means the other thing and that's how we end up with lots of buggy tests. The less magic in tests the better.
I'm not entirely sure why string
, empty_string
and non_empty_string
are off the table. I find them significantly clearer about what they actually mean.
Maybe we could add some other syntactic sugar meaning 'something that doesn't match this condition' and stay away from !
there too. I'd recommend a keyword anything_but
. with clever uses of prototypes this would allow you to write anything_but non_empty_string
.
from test2-suite.
I feel like this thread is getting crazy. @rjbs I don't think anyone is proposing what you are against, though the wording @petdance used does seem to imply that way.
The only thing being proposed is that plain refs without string overloading, ie 'hash(1244)' is almost never what you want. By that a string check on the left that only cares that it is given a string (but does not care what is in it) should not pass if it gets a ref that has no string overloading. in this case you can still do the uncommon case of actually wanting 'hash(1234)' by quoting thr thing on the left.
from test2-suite.
I think @rjbs and I are talking about two different things, and I don't disagree with the things he's talking about at all.
To clarify, Rik, what I was saying is that we want this:
is( $foo, nonblank_string );
to verify that $foo
is \an actual string, that is not an object that just happens to stringify to something.
Right now, in Test::More, if you say
$foo = {};
isnt( $foo, '' );
the test will pass because $foo
stringifies to a HASH(0xBLAHBLAH)
. What we want to say is that in Test2, this test will fail so as to catch the mistake.
$foo = {};
is( $foo, nonblank_string ); # Fails because $foo is not a string, it's a reference.
And if we do want to explicitly check for overloaded stringiness, we can do so like so:
$foo = URI->new( 'http://blahblah.com' );
is( $foo, nonblank_string ); # Fails because $foo is a reference
is( "$foo", nonblank_string ); # Passes because the stringified $foo is a string.
from test2-suite.
I am going to create a milestone for all the various issues here, and split the thing from this ticket into several tickets inside the milestone. I think there are too many discussions here.
from test2-suite.
https://github.com/Test-More/Test2-Suite/milestone/1 created.
from test2-suite.
Sorry, I made a mistake up above. I didn't mean to say that we would change anything about isnt
. I used isnt
by mistake.
Here's corrected version (and I corrected above, too)
$foo = URI->new( 'http://blahblah.com' );
is( $foo, nonblank_string ); # Fails because $foo is a reference
is( "$foo", nonblank_string ); # Passes because the stringified $foo is a string.
from test2-suite.
I think I captured all the issues fairly well in the milestone.
A combination of things mean I probably cannot work much on it this week, hopefully the tickets make a few things clear. It should be possible for others to work many of them if anyone feels like it.
from test2-suite.
I am going to close this ticket in favor of the tickets that came from it. Many of them still reference this ticket making the history easy to find.
from test2-suite.
Related Issues (20)
- Improve documentation for Test2::Tools::Subtest and Test2::AsyncSubtest HOT 1
- Dumper-on-fail for `is`, `isnt`, etc HOT 4
- May want to reconsider exporting `warnings` by default
- Add some number inequality check functions HOT 1
- is_refcount() doesn't support check objects HOT 3
- WITHIN() has exclusionary edges: is(1,within(0,1)) fails
- $CLASS mock within DESCRIBE() affects other tests HOT 4
- Issue with moving Module::Pluggable to optional HOT 3
- Can't locate object method "plugins" via package "Test2::Tools::Tester" HOT 1
- Question: What's a root cause of bundling Importer, Scope::Guard, and Sub::Info? HOT 8
- Test2::Plugin::SRand breaks rand under fork. HOT 1
- Module::Pluggable is a deprecation error on 5.18.4
- Test2::Tools::Compare::{D,DF,F,T etc.) exported by default (contrary to docs) HOT 2
- How to run another test as a sub-process? HOT 4
- Test2::Tools::Compare doc error/typo HOT 1
- Prototype mismatch warning with Test2::Mock HOT 7
- missing docs for 'bool'
- Should srand be disabled by default in Test2::V#? (Was: The -no_rand option is undocumented) HOT 15
- Should blead 'de-CUSTOMIZE` 4 test files changed to accommodate VMS? HOT 2
- Custom comparison operators cannot be used in Test2::Tools::ClassicCompare::cmp_ok
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from test2-suite.