You are viewing a plain text version of this content. The canonical link for it is here.
Posted to test-dev@httpd.apache.org by James G Smith <JG...@TAMU.Edu> on 2004/06/07 22:56:53 UTC
Re: switching t_cmp() argument order
Geoffrey Young <ge...@modperlcookbook.org> wrote:
>hi all
>
>before it gets too late I would like to change t_cmp() to
>
> t_cmp($received, $expected, $comment);
>
>so that test writers can feel at home in either environment.
>
>for the most part the change would be transparent to users as long as their
>tests pass. the only exception is qr// tests, which would error out now and
>force a recode. there's not much we can do about that except to make the
>switch sooner rather than later I guess.
jgsmith@moya:~ % perl -e '$a = qr/this/; print ref $a;' && echo
Regexp
Perhaps do a test and swap the arguments if the first is a Regexp, as
well as issue a deprecation warning.
--
James Smith <JG...@TAMU.Edu>, 979-862-3725
Texas A&M CIS Operating Systems Group, Unix
Re: switching t_cmp() argument order
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>just add a line like, remove after Sep 15th or so. and may be take it into
>>if ($deprecated) { ...}
>>so it's easy to remove the whole branch at once without thinking too much.
>
>
> the logic was already wrapped in an if block, so I just made a note that we
> want it removed by mid september and left it at that. committed, but feel
> free to tweak it more if you like.
no, it's fine
> thanks for the input :)
;)
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: switching t_cmp() argument order
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> just add a line like, remove after Sep 15th or so. and may be take it into
> if ($deprecated) { ...}
> so it's easy to remove the whole branch at once without thinking too much.
the logic was already wrapped in an if block, so I just made a note that we
want it removed by mid september and left it at that. committed, but feel
free to tweak it more if you like.
thanks for the input :)
--Geoff
Re: switching t_cmp() argument order
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
> Stas Bekman wrote:
>
>>Geoffrey Young wrote:
>>
>>
>>>>But it's quite possible that argument could be readonly while not a
>>>>string, a simple example is a return value of a function:
>>>>
>>>>% perl -le 'a(b(), "b"); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}; \
>>>> sub b { 5 }'
>>>>Modification of a read-only value attempted at -e line 1.
>>>
>>>
>>>
>>>I think you need to revisit that example :)
>>
>>
>>I fail to see what do you mean.
>
>
> perl -le 'a(b(), c()); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}; \
> sub b{5}; sub c{6};
I still don't get it. What are you trying to say?
>>>ok, the attached patch reflects that.
>>
>>
>>excellent!
>>
>>the only remaining nit is the deprecation cycle, let's say we happen to
>>release the next few versions within this month, then you hit 1.15
>>really soon. I think it's a matter of time and not release numbers. So
>>may be it's better to say, let's give people some 3 months to move over
>>and set a certain date as a cutoff rather than a version number? Just an
>>idea...
>
>
> sure, we could do that, but then the cutoff isn't really clear. I think
> that three revisions will get us through at least another mod_perl release,
> when people are perhaps likely to glance at the Changes file.
>
> but if we need more time I think we can take it. if the deprecation cycle
> is very long (like 3 new releases takes us a year) I don't think that's
> necessarily a bad thing.
Not for Apache-Test. There will be lots of new mp2 releases in the next few
months to come. Once the API is completed and a few remaining release issues
resolved we will start doing release candidates. So as long as we do at least
one tweak to A-T between these releases I promise you A-T 1.15 before the end
of the summer.
> but either way is fine. I just didn't want it removed in, say, the release
> after the next one.
just add a line like, remove after Sep 15th or so. and may be take it into
if ($deprecated) { ...}
so it's easy to remove the whole branch at once without thinking too much.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: switching t_cmp() argument order
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> Geoffrey Young wrote:
>
>>> But it's quite possible that argument could be readonly while not a
>>> string, a simple example is a return value of a function:
>>>
>>> % perl -le 'a(b(), "b"); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}; \
>>> sub b { 5 }'
>>> Modification of a read-only value attempted at -e line 1.
>>
>>
>>
>> I think you need to revisit that example :)
>
>
> I fail to see what do you mean.
perl -le 'a(b(), c()); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}; \
sub b{5}; sub c{6};
>
>> ok, the attached patch reflects that.
>
>
> excellent!
>
> the only remaining nit is the deprecation cycle, let's say we happen to
> release the next few versions within this month, then you hit 1.15
> really soon. I think it's a matter of time and not release numbers. So
> may be it's better to say, let's give people some 3 months to move over
> and set a certain date as a cutoff rather than a version number? Just an
> idea...
sure, we could do that, but then the cutoff isn't really clear. I think
that three revisions will get us through at least another mod_perl release,
when people are perhaps likely to glance at the Changes file.
but if we need more time I think we can take it. if the deprecation cycle
is very long (like 3 new releases takes us a year) I don't think that's
necessarily a bad thing.
but either way is fine. I just didn't want it removed in, say, the release
after the next one.
--Geoff
Re: switching t_cmp() argument order
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>But it's quite possible that argument could be readonly while not a
>>string, a simple example is a return value of a function:
>>
>>% perl -le 'a(b(), "b"); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}; \
>> sub b { 5 }'
>>Modification of a read-only value attempted at -e line 1.
>
>
> I think you need to revisit that example :)
I fail to see what do you mean.
> ok, the attached patch reflects that.
excellent!
the only remaining nit is the deprecation cycle, let's say we happen to
release the next few versions within this month, then you hit 1.15 really
soon. I think it's a matter of time and not release numbers. So may be it's
better to say, let's give people some 3 months to move over and set a certain
date as a cutoff rather than a version number? Just an idea...
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: switching t_cmp() argument order
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> But it's quite possible that argument could be readonly while not a
> string, a simple example is a return value of a function:
>
> % perl -le 'a(b(), "b"); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}; \
> sub b { 5 }'
> Modification of a read-only value attempted at -e line 1.
I think you need to revisit that example :)
nevertheless, the by-reference nature of $@ was getting in the way of a few
mod_perl tests that I didn't notice before, so I needed to use intermediate
variables anyway :)
> I'd rather see t_is_equal and t_cmp have identical API, if it's not too
> much of a trouble to you.
ok, the attached patch reflects that.
> btw, you don't need to craft \n in t_debug:
> just pass an array and it'll add them for you:
done
--Geoff
Re: switching t_cmp() argument order
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>but the tests need to be adjusted to swap the args, no?
>
>
> only the regex ones, and only before the deprecation period ends.
>
> outside of the regex case, where the comparison order is important, the
> syllogism principle applies.
nice, learned a new term here :)
>>That won't work with readonly args:
>>
>>% perl -le 'a("A", "b"); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}'
>>Modification of a read-only value attempted at -e line 1.
>
>
> yeah, ok. but I don't think this is very common:
>
> t_cmp("a", qr/a/);
>
> which would be the only case where the argument swapping would occur _and_
> that would cause a problem.
>
> so, I can change the patch to use temporary assignments of $@ arguments and
> avoid the problem, or we can agree that comparing a regex to a constant is
> probably never going to happen, or at least it's not a problem if it does,
> since it would force people to recode since they need to anyway.
But it's quite possible that argument could be readonly while not a string, a
simple example is a return value of a function:
% perl -le 'a(b(), "b"); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}; \
sub b { 5 }'
Modification of a read-only value attempted at -e line 1.
>>>+ return t_is_equal($_[1], $_[0]);
>>
>>
>>why the swap of the t_is_equal args?
>
>
> because the regex needs to occur first in t_is_equal(). the new t_cmp()
> expects the regex second (in $expected) so the order needs to be swapped to
> make t_cmp() actually work, since t_is_equal() wants the regex first. I
> could have changed t_is_equal() instead, but I didn't see the need to play
> with two functions when simply changing t_cmp() to match is() (where the
> functionality is essentially the same) was the issue.
I'd rather see t_is_equal and t_cmp have identical API, if it's not too much
of a trouble to you.
btw, you don't need to craft \n in t_debug:
+ my $warning = "WARNING!!! t_cmp() argument order has changed.\n" .
+ "use of a regular expression as the first argument\n" .
+ "is deprecated. support will be removed soon.";
+ t_debug($warning);
just pass an array and it'll add them for you:
t_debug("foo", "bar", "tar");
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: switching t_cmp() argument order
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> outside of the regex case, where the comparison order is important, the
> syllogism principle applies.
whoops, there I go throwing out old terms without dusting off my logic books
(or my head). the logical law I was looking for was symmetry, not
syllogism. go figure :)
--Geoff
Re: switching t_cmp() argument order
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> but the tests need to be adjusted to swap the args, no?
only the regex ones, and only before the deprecation period ends.
outside of the regex case, where the comparison order is important, the
syllogism principle applies.
> That won't work with readonly args:
>
> % perl -le 'a("A", "b"); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}'
> Modification of a read-only value attempted at -e line 1.
yeah, ok. but I don't think this is very common:
t_cmp("a", qr/a/);
which would be the only case where the argument swapping would occur _and_
that would cause a problem.
so, I can change the patch to use temporary assignments of $@ arguments and
avoid the problem, or we can agree that comparing a regex to a constant is
probably never going to happen, or at least it's not a problem if it does,
since it would force people to recode since they need to anyway.
>> t_debug("testing : " . pop) if @_ == 3;
>> - t_debug("expected: " . struct_as_string(0, $_[0]));
>> - t_debug("received: " . struct_as_string(0, $_[1]));
>> - return t_is_equal($_[0], $_[1]);
>> + t_debug("received: " . struct_as_string(0, $_[0]));
>> + t_debug("expected: " . struct_as_string(0, $_[1]));
>> + return t_is_equal($_[1], $_[0]);
>
>
> why the swap of the t_is_equal args?
because the regex needs to occur first in t_is_equal(). the new t_cmp()
expects the regex second (in $expected) so the order needs to be swapped to
make t_cmp() actually work, since t_is_equal() wants the regex first. I
could have changed t_is_equal() instead, but I didn't see the need to play
with two functions when simply changing t_cmp() to match is() (where the
functionality is essentially the same) was the issue.
--Geoff
Re: switching t_cmp() argument order
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
> Stas Bekman wrote:
>
>>+1
>
>
>>+1
>
>
> ok, here's a patch. it warns when t_cmp() is used with a regex and the
> arguments are not in the proper order.
cool, but see below
> the mod_perl test suite seems to run fine with it.
but the tests need to be adjusted to swap the args, no?
> + # this was added in Apache::Test::VERSION 1.12 - remove deprecated
> + # logic in 1.15.
> + if (UNIVERSAL::isa($_[0], 'Regexp')) {
> + my $warning = "WARNING!!! t_cmp() argument order has changed.\n" .
> + "use of a regular expression as the first argument\n" .
> + "is deprecated. support will be removed soon.";
> + t_debug($warning);
> + ($_[0], $_[1]) = ($_[1], $_[0]);
> + }
That won't work with readonly args:
% perl -le 'a("A", "b"); sub a {($_[0], $_[1]) = ($_[1], $_[0]);}'
Modification of a read-only value attempted at -e line 1.
> t_debug("testing : " . pop) if @_ == 3;
> - t_debug("expected: " . struct_as_string(0, $_[0]));
> - t_debug("received: " . struct_as_string(0, $_[1]));
> - return t_is_equal($_[0], $_[1]);
> + t_debug("received: " . struct_as_string(0, $_[0]));
> + t_debug("expected: " . struct_as_string(0, $_[1]));
> + return t_is_equal($_[1], $_[0]);
why the swap of the t_is_equal args?
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: switching t_cmp() argument order
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> +1
> +1
ok, here's a patch. it warns when t_cmp() is used with a regex and the
arguments are not in the proper order.
the mod_perl test suite seems to run fine with it.
--Geoff
Re: switching t_cmp() argument order
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> I'm finding the difference between our t_cmp() and Test::More's is() really
> annoying. that is, whereby t_cmp is
>
> t_cmp($expected, $received, $comment);
>
> is() (and isnt(), and is_deeply(), and maybe others) is
>
> is($received, $expected, $comment);
>
> so when using both I am constantly getting the order mixed up for one or the
> other.
>
> as Test::More has been around longer and is much more popular than we are,
> before it gets too late I would like to change t_cmp() to
>
> t_cmp($received, $expected, $comment);
>
> so that test writers can feel at home in either environment.
>
> for the most part the change would be transparent to users as long as their
> tests pass. the only exception is qr// tests, which would error out now and
> force a recode. there's not much we can do about that except to make the
> switch sooner rather than later I guess.
+1
>>jgsmith@moya:~ % perl -e '$a = qr/this/; print ref $a;' && echo
>>Regexp
>>
>>Perhaps do a test and swap the arguments if the first is a Regexp, as
>>well as issue a deprecation warning.
>
>
> yeah, I thought about that. it's probably a good idea - I'd like to keep
> code working while making things "right" with t_cmp().
+1
but we should take the checking out some time later (let's say a deprecation
cycle of 4-5 releases?) to avoid clutter?
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: switching t_cmp() argument order
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> jgsmith@moya:~ % perl -e '$a = qr/this/; print ref $a;' && echo
> Regexp
>
> Perhaps do a test and swap the arguments if the first is a Regexp, as
> well as issue a deprecation warning.
yeah, I thought about that. it's probably a good idea - I'd like to keep
code working while making things "right" with t_cmp().
I'll work up a patch with this in mind.
--Geoff