Don't complicate things

Every week, I look forward to receiving Gabor's Perl Weekly. I noticed Forking tests in the latest issue. The author has written a password generation module, and wants to ensure that child and parent processes produce different sequences of pseudo-random passwords following a fork. That, in and of itself, is a commendable objective.

However, the author makes the test script unnecessarily complicated, and makes some misleading statements. For example, in closing, the author states:

Windows does not like to fork()

Another easy fix: just skip the test if we're running on windows:

if ( $^O eq 'MSWin32' ) {
     plan( skip_all => 'skip fork tests on MSWin32' ) ;
}

It is true that Perl's fork on Windows does not really do a POSIX fork. However, for stuff like this, you would not notice the difference.

The author also states:

… what I needed to do is to collect the passwords generated in the child process and the parent process and then make sure that they are not the same. I needed what is called IPC … Which is ugly and messy

I played a bit with IPC::Shareable but did not get it working, so I resorted to a simple and battle-tested way to share data between processes: the file system!

I open a temp-file and write each password into this file (in the child and the parent process). After closing all the child processes, I read the file and can now inspect the passwords and make sure they do not repeat.

Yes, IPC can get hairy. But, not this. This is explained very well in perldoc perlipc. For a simple demonstration, see also my crazy stupid regex-redux entry in the Benchmarks Game. That script helped Perl edge ahead of Python3, and runs without modification on Windows and Linux systems (in a sense, it is less stupid than the one which mixes fork with threads).

I decided to install CtrlO::Crypt::XkcdPassword on my Windows testbed. …:

Building and testing Class-Accessor-0.51 ... OK
Successfully installed Class-Accessor-0.51
! Finding WordList (0) on mirror https://cpan.metacpan.org failed.
! Couldn't find module or a distribution WordList
...
! Installing the dependencies failed: Module 'WordList' is not installed
! Bailing out the installation for CtrlO-Crypt-XkcdPassword-1.003.

Well, OK then. I know WordLists::WordList exists, but I am not sure what WordList is ... So, instead of going down that rabbit hole, let me close with a short script which should help the author simplify the tests (and enable them on Windows):

#!/usr/bin/env perl

use strict;
use warnings;

use constant N_RANDOMS => 10;
use Test::More;

run();

sub genrand { int(rand(2**32)) }

sub run {
    my @parents_randoms;
    my @childs_randoms;

    pipe(my $reader, my $writer) or die "Cannot set up pipe: $!";
    my $pid = fork;

    if ( $pid ) {
        close $writer
            or die "Cannot close child's writer in parent: $!";

        @parents_randoms = map genrand(), 1 .. +N_RANDOMS;

        chomp(@childs_randoms = <$reader>);

        close $reader
            or die "Cannot close parent's reader in parent: $!";

        waitpid($pid, 0);
    }
    else {
        defined($pid) or die "Failed to fork: $!";
        close $reader
            or die "Cannot close parent's reader in child: $!";

        srand();
        print $writer genrand(), "\n" for 1 .. +N_RANDOMS;

        close $writer
            or die "Cannot close child's writer in child: $!";
        exit( 0 );
    }

    ok(@parents_randoms == +N_RANDOMS,
        "parent produced the right number of pseudo random integers");
    ok(@childs_randoms == +N_RANDOMS,
        "child produced the right number of pseudo random integers");

    my %all_randoms = map +($_ => 1), @parents_randoms, @childs_randoms;

    ok(keys(%all_randoms) > +N_RANDOMS,
        "parent and child produced different sequences of random integers");

    ok(keys(%all_randoms) == 2 * N_RANDOMS,
        "parent and child produced disjoint sets of pseudo random integers");

    done_testing;
}
__END__

Here is a sample run on Windows 10 with a Visual Studio 2017 built perl 5.26.1:

C:\Users\sinan\AppData\Local\Temp> prove -v --no-color t.pl
t.pl ..
ok 1 - parent produced the right number of pseudo random integers
ok 2 - child produced the right number of pseudo random integers
ok 3 - parent and child produced different sequences of random integers
ok 4 - parent and child produced disjoint sets of pseudo random integers
1..4
ok
All tests successful.
Files=1, Tests=4,  0 wallclock secs ( 0.06 usr +  0.00 sys =  0.06 CPU)
Result: PASS

There is a small probability (I didn't bother to calculate) that the last test will fail just due to randomness. In fact, one might consider it sufficient if the generated sequences just differ instead of being completely disjoint. I expect more from my pseudo-random number generators ;-)

If you are using an older perl on Windows, don't use builtin rand.

Test driven development is great in principle, but in all too many instances, the tests themselves can be the source of problems. Simplicity is important, especially in tests.

Premptive response: No, I am not going to put together a pull request. There are too many nitty gritty details to get into. If the author wants improve the module's code and tests based on the example above, he/she can do it with an acknowledgement. If others reading this post are prompted to think "maybe I can make this piece of code simpler" as a result of comparing and contrasting the snippet above with the significantly more complicated original test script, that's even better.

As a side note, I still run into modules that try to create temporary files in the root directory of my C: drive. That usually happens due to the script clearing the environment and not saving temporary directory locations. This is an unfortunate interaction with File::Spec->tmpdir which defaults to trying to write to the root (hey, Windows 95 allowed it!) of the current drive if it can't locate the customary directories. I think File::Spec->tmpdir ought to croak if the environment does not contain one of TMP, TEMP, or TMPDIR, instead of offering C:\system\temp or C:\temp or /tmp or / on Windows. Regardless of File::Spec's behavior, scripts, modules, etc should not delete those environment variables.

You can discuss this post on r/perl.