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

t/op/not.t: Misspecified tests; add descriptions to tests lacking them #12676

Closed
p5pRT opened this issue Dec 29, 2012 · 8 comments
Closed

t/op/not.t: Misspecified tests; add descriptions to tests lacking them #12676

p5pRT opened this issue Dec 29, 2012 · 8 comments

Comments

@p5pRT
Copy link

p5pRT commented Dec 29, 2012

Migrated from rt.perl.org#116242 (status was 'resolved')

Searchable as RT116242$

@p5pRT
Copy link
Author

p5pRT commented Dec 29, 2012

From @jkeenan

Since the St Louis Perl Hackathon in November, I and others have been
writing descriptions for tests in the Perl 5 core distribution's test
suite which currently lack them. I have encountered a case where adding
descriptions to two tests (a) in one case changes the outcome of the
test from PASS to FAIL; and (b) where in both cases the descriptions
fail to print -- as if they had never been written in the first place!

Consider tests 4 and 5 in t/op/not.t​:

#####
# test not(..) and !
is(! 1, not 1);
is(! 0, not 0);
#####

Currently, none of the tests in t/op/not.t have descriptions. Hence,
the output of running these two tests is as opaque as that of any other
test in that file​:

#####
$ ./perl t/op/not.t
1..16
ok 1
...
ok 4
ok 5
...
ok 16
#####

Now, suppose we add descriptions to all tests in this file. Here I
display only the first five​:

#####
pass("logical negation of empty list") if not();
is(not(), 1, "logical negation of empty list in numeric comparison");
is(not(), not(0),
  "logical negation of empty list compared with logical negation of
false value");

is(! 1, not 1, q{description 1});
is(! 0, not 0, q{description 0});
#####

The output I get is​:

#####
$ ./perl t/op/xnot.t
1..5
ok 1 - logical negation of empty list
ok 2 - logical negation of empty list in numeric comparison
ok 3 - logical negation of empty list compared with logical negation of
false value
ok 4
not ok 5
# Failed test 5 - at t/op/xnot.t line 19
# got "1"
# expected ""
#####

Where did the descriptions for tests 4 and 5 go? And why is test 5 now
failing?

I posed this question on the St Louis and Toronto Perlmongers mailing
lists. Thanks to Nathan Nutter, Scott Smith, Liam R E Quin, Olaf
Alders, Uri Guttman, Tom Legrady, Fulko Hew for participating in the
discussion.

Adapting a diagnostic suggested by Fulko Hew, let's set aside the
"testing" aspect of the problem and define a function which prints out
its arguments​:

#####
sub examine {print "arg​: <$_>\n" for @​_;}
#####

Let's first pass to this function the 3 arguments provided to is()
above, but without the 'not ' in the second argument​:

#####
examine(! 1, 1, q{description 1});
examine(! 0, 0, q{description 0});
#####

The output​:

#####
arg​: <>
arg​: <1>
arg​: <description 1>
arg​: <1>
arg​: <0>
arg​: <description 0>
#####

The high-precedence '!' operator binds to the number following it and
(apparently) immediately forces that argument to be evaluated. '! 1'
evaluates to Perl-false -- hence the empty string between the
angle-brackets in the first line of output. '! 0' evaluates to
Perl-true -- hence the '1' in the fourth line of output. As we would
intuitively expect, examine() reports 3 arguments in each call. And why
do we intuitively expect that? Because the commas inside the function
call are functioning as item separators within a list.

Now let's add the string 'not ' to the second argument in each call​:

#####
print "Prepending 'not ' to second argument\n";
examine(! 1, not 1, q{description 1});
examine(! 0, not 0, q{description 0});
#####

This time, our overall output is​:
#####
arg​: <>
arg​: <1>
arg​: <description 1>
arg​: <1>
arg​: <0>
arg​: <description 0>
Prepending 'not ' to second argument
arg​: <>
arg​: <>
arg​: <1>
arg​: <>
#####

Here we're seeing the apparently anomalous results we saw when we tried
to add a description to tests 4 and 5 in t/op/not.t. We note​:

1. The high-precedence '!' operator is continuing to bind to the number
following it in each call, forcing immediate evaluation. The first and
third lines following "Prepending" correspond to the first and fourth
lines above "Prepending".

2. Each call, however, is only reporting two "arguments" -- and each
"argument" is being evaluated to an empty string (second and fourth
lines following "Prepending").

In the discussion on the Perlmonger mailing lists, Uri Guttman observed
that 'not' isn't just a lower precedence version of !. It is also a
named function with a prototype.

#####
./perl -le 'print prototype "CORE​::not"'
$;
#####

Since 'not' as a function is prototyped to expect a scalar argument, the
comma to the right of each of 'not 1' and 'not 0' is no longer
functioning as an item separator in list context. Rather (to quote
'perlop'), "[i]n scalar context it evaluates its left argument, throws
that value away, then evaluates its right argument and returns that
value." The "right argument", in these cases, are q{description 1} and
q{description 0}. When evaluated by 'not' -- now serving as a unary
negation operator -- the expressions evaluate to Perl-false. Hence the
empty strings between the angle brackets in the second and fourth lines
after "Prepending".

If you adapt the diagnostic suggested by Fulko to use either
Test​::More​::is() or the is() from 't/test.pl', you should get similar
results.

This suggests that tests in 4 and 5 in t/op/not.t are not conducting a
precise test of what you would at first think they are testing. As
written, you would think they're testing that '! 1' and 'not 1' "do the
same thing" and that '! 0' and 'not 0' "do the same thing" as well. But
the fact that adding a description to the test affects the outcome of
the test when run in a testing context means that the tests are not
spelled sufficiently precisely.

Hence, I propose we apply the two patches attached. The first merely
puts parentheses around the second argument in each of tests 4 and 5.
The second adds descriptions to those two tests and to every other test
in the file as well.

Please review.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Dec 29, 2012

From @jkeenan

0001-Add-parens-around-second-argument-to-tests-4-and-5.patch
From 22adfd31e44fba1a05f3c8a238c1e6a94539996a Mon Sep 17 00:00:00 2001
From: James E Keenan <jkeenan@cpan.org>
Date: Sat, 29 Dec 2012 11:09:34 -0500
Subject: [PATCH 1/2] Add parens around second argument to tests 4 and 5.

If a description were to be added to these tests, in the absence of
parentheses the scalar prototype of CORE::not would enforce a scalar context
onto the balance of the statement, leading to apparently anomalous behavior,
viz., the descriptions would not be printed and test 5 would be reported to
FAIL.
---
 t/op/not.t |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/op/not.t b/t/op/not.t
index 3d07797..aa3e192 100644
--- a/t/op/not.t
+++ b/t/op/not.t
@@ -14,8 +14,8 @@ is(not(), 1);
 is(not(), not(0));
 
 # test not(..) and !
-is(! 1, not 1);
-is(! 0, not 0);
+is(! 1, (not 1));
+is(! 0, (not 0));
 is(! (0, 0), not(0, 0));
 
 # test the return of !
-- 
1.6.3.2

@p5pRT
Copy link
Author

p5pRT commented Dec 29, 2012

From @jkeenan

0002-t-op-not.t-Add-descriptions-to-all-tests.patch
From 68b3dfb6c35650d77142c62c9aad62acea4c7b4c Mon Sep 17 00:00:00 2001
From: James E Keenan <jkeenan@cpan.org>
Date: Sat, 29 Dec 2012 11:23:56 -0500
Subject: [PATCH 2/2] t/op/not.t:  Add descriptions to all tests.

Also, add note() before tests 4 and 5 explaining rationale for addition of
parentheses to second arguments.
---
 t/op/not.t |   47 +++++++++++++++++++++++++++++++----------------
 1 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/t/op/not.t b/t/op/not.t
index aa3e192..4aae02c 100644
--- a/t/op/not.t
+++ b/t/op/not.t
@@ -9,14 +9,19 @@ BEGIN {
 plan tests => 16;
 
 # not() tests
-pass() if not();
-is(not(), 1);
-is(not(), not(0));
+pass("logical negation of empty list") if not();
+is(not(), 1, "logical negation of empty list in numeric comparison");
+is(not(), not(0),
+    "logical negation of empty list compared with logical negation of false value");
 
 # test not(..) and !
-is(! 1, (not 1));
-is(! 0, (not 0));
-is(! (0, 0), not(0, 0));
+note("parens needed around second argument in next two tests\nto preserve list context inside function call");
+is(! 1, (not 1),
+    "high- and low-precedence logical negation of true value");
+is(! 0, (not 0),
+    "high- and low-precedence logical negation of false value");
+is(! (0, 0), not(0, 0),
+    "high- and low-precedence logical negation of lists");
 
 # test the return of !
 {
@@ -24,13 +29,18 @@ is(! (0, 0), not(0, 0));
     my $not1 = ! 1;
 
     no warnings;
-    ok($not1 == undef);
-    ok($not1 == ());
+    ok($not1 == undef,
+        "logical negation (high-precedence) of true value is numerically equal to undefined value");
+    ok($not1 == (),
+        "logical negation (high-precedence) of true value is numerically equal to empty list");
 
     use warnings;
-    ok($not1 eq '');
-    ok($not1 == 0);
-    ok($not0 == 1);
+    ok($not1 eq '',
+        "logical negation (high-precedence) of true value in string context is equal to empty string");
+    ok($not1 == 0,
+        "logical negation (high-precedence) of true value is false in numeric context");
+    ok($not0 == 1,
+        "logical negation (high-precedence) of false value is true in numeric context");
 }
 
 # test the return of not
@@ -39,11 +49,16 @@ is(! (0, 0), not(0, 0));
     my $not1 = not 1;
 
     no warnings;
-    ok($not1 == undef);
-    ok($not1 == ());
+    ok($not1 == undef,
+        "logical negation (low-precedence) of true value is numerically equal to undefined value");
+    ok($not1 == (),
+        "logical negation (low-precedence) of true value is numerically equal to empty list");
 
     use warnings;
-    ok($not1 eq '');
-    ok($not1 == 0);
-    ok($not0 == 1);
+    ok($not1 eq '',
+        "logical negation (low-precedence) of true value in string context is equal to empty string");
+    ok($not1 == 0,
+        "logical negation (low-precedence) of true value is false in numeric context");
+    ok($not0 == 1,
+        "logical negation (low-precedence) of false value is true in numeric context");
 }
-- 
1.6.3.2

@p5pRT
Copy link
Author

p5pRT commented Jan 3, 2013

From @tonycoz

On Sat Dec 29 08​:29​:43 2012, jkeen@​verizon.net wrote​:

Hence, I propose we apply the two patches attached. The first merely
puts parentheses around the second argument in each of tests 4 and 5.
The second adds descriptions to those two tests and to every other test
in the file as well.

Please review.

That all makes sense to me.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jan 3, 2013

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jan 5, 2013

From @jkeenan

On Wed Jan 02 19​:37​:18 2013, tonyc wrote​:

On Sat Dec 29 08​:29​:43 2012, jkeen@​verizon.net wrote​:

Hence, I propose we apply the two patches attached. The first merely
puts parentheses around the second argument in each of tests 4 and 5.
The second adds descriptions to those two tests and to every other test
in the file as well.

Please review.

That all makes sense to me.

Tony

Thanks for your feedback.

Applied to blead in these commits​:

commit 129da47
  Add parens around second argument to tests 4 and 5.

commit 877c9ac
  t/op/not.t​: Add descriptions to all tests.

Resolving ticket.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Jan 5, 2013

@jkeenan - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed Jan 5, 2013
@p5pRT
Copy link
Author

p5pRT commented Jan 22, 2013

From @ap

* James E Keenan <perlbug-followup@​perl.org> [2012-12-29 17​:35]​:

In the discussion on the Perlmonger mailing lists, Uri Guttman
observed that 'not' isn't just a lower precedence version of !.
It is also a named function with a prototype.

#####
./perl -le 'print prototype "CORE​::not"'
$;
#####

Ugh. Whyever is the prototype `$;` rather than just `$`?

The latter would do exactly what one would expect here​:

  $ perl -E 'say not 0, "foo"'

  $ perl -E 'sub non ($) { !$_[0] } say non 0, "foo"'
  1foo

There must be hysterical raisins for this nonsense, I’m sure… it
is not just a stupid mistake, hopefully, is it? Anyone know why?

Regards,
--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

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

1 participant