Good code, bad tests

I've been working on IO::Async::SSL recently. Both the previous upload, and the next one I shall shortly do, are fixes for failing smoke tests. Moreover, they're fixes for failing tests, of correct code.

The first concerns the semantics of the END block. The code looked roughly like

use Test::More
system( 'socat -help' ) == 0 or plan skip_all => "No socat";
plan tests => 3;

open my $kid, "-|", "socat", .... or die;
END { kill TERM => $kid }

The idea of this code was to ensure that socat is no longer running when the test exits. Of course, if the socat program isn't installed, the entire test is skipped. And the END block is still run. The result of this is that since $kid is undefined, the containing perl process is sent SIGTERM instead, and the test harness exits with an error. Ooops.

Now what I actually wanted here was

END { kill TERM => $kid if defined $kid }

I wonder if this situation would warrant a new block-alike, perhaps lets call it ENDX, which is only executed at END time if the line declaring it was actually reached in code. Perhaps it can be hacked up by

my @ends;
sub ENDX(&) {
   push @ends, $_[0];
}
END { $_->() for @ends }
...
ENDX { kill TERM => $kid };

The second failure is a curious one that starts off looking like an OS-specific bug in the code. All the Linux smoke boxes were giving fails on a particularly innocent-looking test

is( $client->peeraddr, $server->sockaddr, 'Client socket address' );

Under closer inspection, it seems that the way Linux returns socket addresses from the kernel doesn't initialise the "holes" in the address structure, whereas most other OSes do.

The result of this is that rendered as strings of bytes, the two addresses don't necessarily contain all the same bytes as each other, even though they represent the same address. The way to fix this one is to unpack the addresses, known to be AF_INET addresses, and use is_deeply instead:

use Socket qw( unpack_sockaddr_in );

is_deeply( [ unpack_sockaddr_in $client->peeraddr ],
           [ unpack_sockaddr_in $server->sockaddr ],
           'Client socket address' );

This doesn't look too neat this way, but perhaps there's some scope for considering a Test::Sockaddr module or somesuch, to neaten it up. Perhaps a little special-purpose though..