search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 559 forks source link

Refactor t/op/lop.t to use test.pl instead of making TAP by hand #12300

Closed p5pRT closed 11 years ago

p5pRT commented 12 years ago

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

Searchable as RT114296$

p5pRT commented 12 years ago

From @perlDreamer

I stayed away from using done_testing in this patch since someone corrected one of the earlier ones where I did use it.

Colin

p5pRT commented 12 years ago

From @perlDreamer

0009-Update-t-op-lop.t-to-use-test.pl-instead-of-making-T.patch ```diff From b35d8d5f3fdbf2c5c0401a35bc02727e0afa1cb1 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Sat, 28 Jul 2012 14:14:45 -0700 Subject: [PATCH 9/9] Update t/op/lop.t to use test.pl instead of making TAP by hand. --- t/op/lop.t | 29 ++++++++++++----------------- 1 files changed, 12 insertions(+), 17 deletions(-) diff --git a/t/op/lop.t b/t/op/lop.t index 2c2d2a6..c11fd92 100644 --- a/t/op/lop.t +++ b/t/op/lop.t @@ -7,9 +7,10 @@ BEGIN { chdir 't' if -d 't'; @INC = '../lib'; + require './test.pl'; } -print "1..11\n"; +plan tests => 17; my $test = 0; for my $i (undef, 0 .. 2, "", "0 but true") { @@ -29,37 +30,31 @@ for my $i (undef, 0 .. 2, "", "0 but true") { and (($i || !$j) != (!$i && $j)) ); } - if (not $true) { - print "not "; - } elsif ($false) { - print "not "; - } - print "ok ", ++$test, "\n"; + my $m = ! defined $i ? 'undef' + : $i eq '' ? 'empty string' + : $i; + ok( $true, "true: $m"); + ok( ! $false, "false: $m"); } # $test == 6 my $i = 0; (($i ||= 1) &&= 3) += 4; -print "not " unless $i == 7; -print "ok ", ++$test, "\n"; +is( $i, 7 ); my ($x, $y) = (1, 8); $i = !$x || $y; -print "not " unless $i == 8; -print "ok ", ++$test, "\n"; +is( $i, 8 ); ++$y; $i = !$x || !$x || !$x || $y; -print "not " unless $i == 9; -print "ok ", ++$test, "\n"; +is( $i, 9 ); $x = 0; ++$y; $i = !$x && $y; -print "not " unless $i == 10; -print "ok ", ++$test, "\n"; +is( $i, 10 ); ++$y; $i = !$x && !$x && !$x && $y; -print "not " unless $i == 11; -print "ok ", ++$test, "\n"; +is( $i, 11 ); -- 1.7.5.4 ```
p5pRT commented 12 years ago

From @jkeenan

On Sat Jul 28 14​:21​:04 2012\, colink@​perldreamer.com wrote​:

I stayed away from using done_testing in this patch since someone corrected one of the earlier ones where I did use it.

Colin

I'm glad that you added content to the names/labels/descriptions -- call them what you will -- of the first dozen tests. But tests 13 through 18 in the patched file have no descriptions other than their sequence number. Would it be possible to give them some meaningful descriptions as well?

Thank you very much. Jim Keenan

p5pRT commented 12 years ago

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

p5pRT commented 12 years ago

From @perlDreamer

Here's a supplementary patch for that.

p5pRT commented 12 years ago

From @perlDreamer

0010-Document-that-last-five-tests.patch ```diff From 8afb4d4c62e367229c6de2198606d1c272dd0bbe Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Fri, 10 Aug 2012 20:24:09 -0700 Subject: [PATCH 10/10] Document that last five tests. --- t/op/lop.t | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/op/lop.t b/t/op/lop.t index c11fd92..1ce8c46 100644 --- a/t/op/lop.t +++ b/t/op/lop.t @@ -40,21 +40,21 @@ for my $i (undef, 0 .. 2, "", "0 but true") { # $test == 6 my $i = 0; (($i ||= 1) &&= 3) += 4; -is( $i, 7 ); +is( $i, 7, '||=, &&='); my ($x, $y) = (1, 8); $i = !$x || $y; -is( $i, 8 ); +is( $i, 8, 'negation precedence with ||' ); ++$y; $i = !$x || !$x || !$x || $y; -is( $i, 9 ); +is( $i, 9, 'negation precedence with ||, multiple operands' ); $x = 0; ++$y; $i = !$x && $y; -is( $i, 10 ); +is( $i, 10, 'negation precedence with &&' ); ++$y; $i = !$x && !$x && !$x && $y; -is( $i, 11 ); +is( $i, 11, 'negation precedence with &&, multiple operands' ); -- 1.7.5.4 ```
p5pRT commented 12 years ago

From @jkeenan

On Fri Aug 10 20​:43​:03 2012\, colink@​perldreamer.com wrote​:

Here's a supplementary patch for that.

When I applied patches 0009 and 0010 in succession\, I got one commented-out line that should probably be removed​:

  40 # $test == 6   41 my $i = 0;   42 (($i ||= 1) &&= 3) += 4;   43 is( $i\, 7\, '||=\, &&=');

And I also got two lines at the end which don't seem to feed into any test.

  49 ++$y;   50 $i = !$x || !$x || !$x || $y;   51 is( $i\, 9\, 'negation precedence with ||\, multiple operands' );   52   53 $x = 0;   54 ++$y;

Thank you very much. Jim Keenan

p5pRT commented 12 years ago

From @perlDreamer

I can give you another patch which removes that comment line\, but it was there before I started tweaking the test.

I also downloaded the two patches and applied them to a fresh copy of blead\, and they were clean. There's extra code either below the fold\, or down a screen.

I'll attach the combined patch and the patched result for comparison.

p5pRT commented 12 years ago

From @perlDreamer

#!./perl

# # test the logical operators '&&'\, '||'\, '!'\, 'and'\, 'or'\, 'not' #

BEGIN {   chdir 't' if -d 't';   @​INC = '../lib';   require './test.pl'; }

plan tests => 17;

my $test = 0; for my $i (undef\, 0 .. 2\, ""\, "0 but true") {   my $true = 1;   my $false = 0;   for my $j (undef\, 0 .. 2\, ""\, "0 but true") {   $true &&= !(   ((!$i || !$j) != !($i && $j))   or (!($i || $j) != (!$i && !$j))   or (!!($i || $j) != !(!$i && !$j))   or (!(!$i || !$j) != !!($i && $j))   );   $false ||= (   ((!$i || !$j) == !!($i && $j))   and (!!($i || $j) == (!$i && !$j))   and ((!$i || $j) == ($i && !$j))   and (($i || !$j) != (!$i && $j))   );   }   my $m = ! defined $i ? 'undef'   : $i eq '' ? 'empty string'   : $i;   ok( $true\, "true​: $m");   ok( ! $false\, "false​: $m"); }

# $test == 6 my $i = 0; (($i ||= 1) &&= 3) += 4; is( $i\, 7\, '||=\, &&=');

my ($x\, $y) = (1\, 8); $i = !$x || $y; is( $i\, 8\, 'negation precedence with ||' );

++$y; $i = !$x || !$x || !$x || $y; is( $i\, 9\, 'negation precedence with ||\, multiple operands' );

$x = 0; ++$y; $i = !$x && $y; is( $i\, 10\, 'negation precedence with &&' );

++$y; $i = !$x && !$x && !$x && $y; is( $i\, 11\, 'negation precedence with &&\, multiple operands' );

p5pRT commented 12 years ago

From @perlDreamer

op_lop_t_full.patch ```diff diff --git a/t/op/lop.t b/t/op/lop.t index 2c2d2a6..1ce8c46 100644 --- a/t/op/lop.t +++ b/t/op/lop.t @@ -7,9 +7,10 @@ BEGIN { chdir 't' if -d 't'; @INC = '../lib'; + require './test.pl'; } -print "1..11\n"; +plan tests => 17; my $test = 0; for my $i (undef, 0 .. 2, "", "0 but true") { @@ -29,37 +30,31 @@ for my $i (undef, 0 .. 2, "", "0 but true") { and (($i || !$j) != (!$i && $j)) ); } - if (not $true) { - print "not "; - } elsif ($false) { - print "not "; - } - print "ok ", ++$test, "\n"; + my $m = ! defined $i ? 'undef' + : $i eq '' ? 'empty string' + : $i; + ok( $true, "true: $m"); + ok( ! $false, "false: $m"); } # $test == 6 my $i = 0; (($i ||= 1) &&= 3) += 4; -print "not " unless $i == 7; -print "ok ", ++$test, "\n"; +is( $i, 7, '||=, &&='); my ($x, $y) = (1, 8); $i = !$x || $y; -print "not " unless $i == 8; -print "ok ", ++$test, "\n"; +is( $i, 8, 'negation precedence with ||' ); ++$y; $i = !$x || !$x || !$x || $y; -print "not " unless $i == 9; -print "ok ", ++$test, "\n"; +is( $i, 9, 'negation precedence with ||, multiple operands' ); $x = 0; ++$y; $i = !$x && $y; -print "not " unless $i == 10; -print "ok ", ++$test, "\n"; +is( $i, 10, 'negation precedence with &&' ); ++$y; $i = !$x && !$x && !$x && $y; -print "not " unless $i == 11; -print "ok ", ++$test, "\n"; +is( $i, 11, 'negation precedence with &&, multiple operands' ); ```
p5pRT commented 12 years ago

From @nwc10

On Sat\, Jul 28\, 2012 at 02​:21​:05PM -0700\, Colin Kuskie wrote​:

# New Ticket Created by Colin Kuskie # Please include the string​: [perl #114296] # in the subject line of all future correspondence about this issue. # \<URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=114296 >

I stayed away from using done_testing in this patch since someone corrected one of the earlier ones where I did use it.

We don't all agree on everything. I personally tend to find it easier (because I hate repeatedly re-editing test conts\, and I guess historically I also got burned by them often conflicting on maint-branch integrations)

From b35d8d5f3fdbf2c5c0401a35bc02727e0afa1cb1 Mon Sep 17 00​:00​:00 2001 From​: Colin Kuskie \colink@&#8203;perldreamer\.com Date​: Sat\, 28 Jul 2012 14​:14​:45 -0700 Subject​: [PATCH 9/9] Update t/op/lop.t to use test.pl instead of making TAP by hand.

Thanks applied as 73ee8f56b01b1a87f68b11ab07bda9013c14f039

Nicholas Clark

p5pRT commented 12 years ago

From @nwc10

On Fri\, Aug 10\, 2012 at 08​:43​:04PM -0700\, Colin Kuskie via RT wrote​:

Here's a supplementary patch for that.

I applied this as 6f02a29952bee8a2f755ecdff8223ec5db30a97d\, but tweaked the commit message.

Nicholas Clark

p5pRT commented 12 years ago

From @nwc10

On Sat\, Aug 11\, 2012 at 06​:50​:56AM -0700\, James E Keenan via RT wrote​:

On Fri Aug 10 20​:43​:03 2012\, colink@​perldreamer.com wrote​:

Here's a supplementary patch for that.

When I applied patches 0009 and 0010 in succession\, I got one commented-out line that should probably be removed​:

 40 \# $test == 6

I removed that in commit 28453e5f3d3167d0f1a72abc434059ee99c2ef96\, along with the declaration of my $test

And I also got two lines at the end which don't seem to feed into any test.

 49 \+\+$y;
 50 $i = \!$x || \!$x || \!$x || $y;
 51 is\( $i\, 9\, 'negation precedence with ||\, multiple operands' \);
 52 
 53 $x = 0;
 54 \+\+$y;

I missed this.

What's the right fix here?

[not going to mark this ticket as resolved just yet]

Nicholas Clark

p5pRT commented 12 years ago

From @perlDreamer

 52
 53 $x = 0;
 54 \+\+$y;

I missed this.

What's the right fix here?

[not going to mark this ticket as resolved just yet]

Those lines (lines 52-54 after both patches are applied) lead into another test​:

51 $x = 0; 52 ++$y; 53 $i = !$x && $y; 54 is( $i\, 10\, 'negation precedence with &&' );

Note\, there's a 2 line offset between the original patch and this code due to removing a comment and a blank line.

p5pRT commented 12 years ago

From @iabyn

On Wed\, Aug 29\, 2012 at 09​:36​:18PM +0100\, Nicholas Clark wrote​:

On Sat\, Jul 28\, 2012 at 02​:21​:05PM -0700\, Colin Kuskie wrote​:

I stayed away from using done_testing in this patch since someone corrected one of the earlier ones where I did use it.

We don't all agree on everything. I personally tend to find it easier (because I hate repeatedly re-editing test conts\, and I guess historically I also got burned by them often conflicting on maint-branch integrations)

As the 5.10.1 maint pumpking\, I also hated all the merge conflicts caused by updates to the test count. However\, I *really* hate the idea that some tests are quietly dying or misbehaving or being skipped\, and we never notice.

I've wondered from time to time whether the test infrastructure could be made to deduce the test count from code comments\, e.g.

  plan(use_comments);

  # TESTS=2   {   is(...);   is(...);   }

  # TESTS=3   {   is(...);   is(...);   is(...);   }

so plan() scans the source file and sums all the TESTS=N.

-- In my day\, we used to edit the inodes by hand. With magnets.

p5pRT commented 12 years ago

From @Leont

On Wed\, Aug 29\, 2012 at 10​:36 PM\, Nicholas Clark \nick@&#8203;ccl4\.org wrote​:

We don't all agree on everything. I personally tend to find it easier (because I hate repeatedly re-editing test conts\, and I guess historically I also got burned by them often conflicting on maint-branch integrations)

If it is uncertain all tests are run (for example if some are in a signal handler)\, it is very important to have that count explicitly\, or else you won't notice missing tests. I think the problem was that you used the simple countless version. When used with a count\, it offers the same guarantees as plan does\, with the additional benefit that you can postpone planning. E.G.

$count += 3 # This should be 3 tests

$count += 6 # 6 tests

done_testing($count);

Leon

p5pRT commented 12 years ago

From @demerphq

On 14 September 2012 13​:34\, Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

On Wed\, Aug 29\, 2012 at 09​:36​:18PM +0100\, Nicholas Clark wrote​:

On Sat\, Jul 28\, 2012 at 02​:21​:05PM -0700\, Colin Kuskie wrote​:

I stayed away from using done_testing in this patch since someone corrected one of the earlier ones where I did use it.

We don't all agree on everything. I personally tend to find it easier (because I hate repeatedly re-editing test conts\, and I guess historically I also got burned by them often conflicting on maint-branch integrations)

As the 5.10.1 maint pumpking\, I also hated all the merge conflicts caused by updates to the test count. However\, I *really* hate the idea that some tests are quietly dying or misbehaving or being skipped\, and we never notice.

I've wondered from time to time whether the test infrastructure could be made to deduce the test count from code comments\, e.g.

plan\(use\_comments\);

\# TESTS=2
\{
    is\(\.\.\.\);
    is\(\.\.\.\);
\}

\# TESTS=3
\{
    is\(\.\.\.\);
    is\(\.\.\.\);
    is\(\.\.\.\);
\}

so plan() scans the source file and sums all the TESTS=N.

Some of the regex tests used to do this via a BEGIN.

BEGIN { $testcount+=3 }

and then the $testcount was used in the plan.

But the code police decided this was bad style and patched it away and I didn't notice until it was pretty much done. I always thought it was a better solution than a hard coded test count.

cheers\, Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 12 years ago

From @shlomif

Hi Dave\,

On Fri\, 14 Sep 2012 12​:34​:46 +0100 Dave Mitchell \davem@&#8203;iabyn\.com wrote​:

On Wed\, Aug 29\, 2012 at 09​:36​:18PM +0100\, Nicholas Clark wrote​:

On Sat\, Jul 28\, 2012 at 02​:21​:05PM -0700\, Colin Kuskie wrote​:

I stayed away from using done_testing in this patch since someone corrected one of the earlier ones where I did use it.

We don't all agree on everything. I personally tend to find it easier (because I hate repeatedly re-editing test conts\, and I guess historically I also got burned by them often conflicting on maint-branch integrations)

As the 5.10.1 maint pumpking\, I also hated all the merge conflicts caused by updates to the test count. However\, I *really* hate the idea that some tests are quietly dying or misbehaving or being skipped\, and we never notice.

I've wondered from time to time whether the test infrastructure could be made to deduce the test count from code comments\, e.g.

plan\(use\_comments\);

\# TESTS=2
\{
is\(\.\.\.\);
is\(\.\.\.\);
\}

\# TESTS=3
\{
is\(\.\.\.\);
is\(\.\.\.\);
is\(\.\.\.\);
\}

so plan() scans the source file and sums all the TESTS=N.

This is the premise of my Test-Count CPAN distribution​:

https://metacpan.org/module/Test::Count

Although what I do there is rely on an external process to process the code and update the count based on the annotations rather than on the plan scanning the source at every time.

Regards\,

  Shlomi Fish

--

Shlomi Fish http​://www.shlomifish.org/ What does "Zionism" mean? - http​://shlom.in/def-zionism

Trying to block Internet pornography is like climbing a waterfall and trying to stay dry. (— one of Shlomi Fish’s Internet Friends)

Please reply to list if it's a mailing list post - http​://shlom.in/reply .

p5pRT commented 11 years ago

From @jkeenan

The discussion in this ticket wandered off from its original scope​: Colin's refactoring of the test file to use t/test.pl.

AFAICT\, the tests have been working properly for months\, so I am closing this ticket. If anyone would like to continue discussing the best way to count tests\, I recommend writing P5P.

Colin\, thanks for your efforts on the test suite\, which have inspired myself and others!

Thank you very much. Jim Keenan

p5pRT commented 11 years ago

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