Search code examples
perlunit-testinghashreturn-typehash-of-hashes

Unit testing in perl, Receiving hash ref as return expected to return a string from a key in a hash


I am trying to test the output of the following method:

package ASC::Builder::Error;

sub new {

    my ($package, $first_param) = (shift, shift);


    if (ref $first_param eq 'HASH') {
        my %params = @_;
        return bless { message => $first_param->{message}, %params}, $package;
    }
    else {
        my %params = @_; 
        return bless {message => $first_param, %params}, $package;

}   
}

This method is supposed to accept either an error hash or error string. If it accepts a hash it should output the value of the message key from the error hash.

This is the error hash located in ErrorLibrary.pm:

 use constant {

        CABLING_ERROR => {
        code => 561,
        message => "cabling is not correct at T1",
        tt => { template => 'disabled'},
        fatal => 1,
        link =>'http://www.e-solution.com/CABLING_ERROR',
        },
    };

This is the message method along with the other keys of the hash located in Error.pm

package ASC::Builder::Error;

sub message {
        return $_[0]->{message}; 
}

sub tt {
    return {$_[0]->{tt} };
}

sub code {
    return {$_[0]->{code} };
}

This is my current unit test located in error.t

#input value will either be a String or and Error Message Hash


# error hash
my $error_hash = CABLING_ERROR;
# error string
my $error_string = "cabling is not correct at T1.";

# error hash is passed into new and an error object is outputted
my $error_in = ASC::Builder::Error->new($error_hash);

# checks to see if the output object from new is an Error object
isa_ok($error_in, 'ASC::Builder::Error');

# checking that object can call the message() method
can_ok( $error_in, 'message');


# checks to see if the output message matches the message contained in the error hash(correct)
is($error_in->message(),( $error_string || $error_hash->{message} ), 'Returns correct error message');

And finally the results of my test:

#   Failed test 'Returns correct error message'
#   at t/67_error_post.t line 104.
#          got: 'HASH(0x38b393d490)'
#     expected: 'cabling is not correct at T1.'
# 
# '
# Looks like you failed 1 test of 3.
t/67_error_post.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/3 subtests 

Solution

  • On my machine

    First of, if I run your code I get an error about CABLING_CHECK_TOR_INCORRECT_CABLING_ERROR being not defined. If I replace that with CABLING_ERROR, the test fails with this.

    #          got: 'cabling is not correct at T1'
    #     expected: 'cabling is not correct at T1.'
    # Looks like you failed 1 test of 3.
    

    Two possible outputs at the same time

    Now to what you say the output is.

    For some reason, your $error_in->message returns a hashref, which gets stringified by is(), because is() doesn't do data structures. You can use Test::Deep to do this.

    use Test::Deep;
    
    cmp_deeply(
        $error_in->message,
        any(
            $error_string,
            $error_hash->{message},
        ),
        'Returns correct error message',
    );
    

    Here I assumed that your $error_string || $error_hash->{message} is intended to make it check for either one or the other.

    But || will just check if $error_string has a true value and return it, or take the value of $error_hash->{message}. It compares the result of that operation to $error_in->message.

    Testing clearly

    However, this will likely not solve your real problem. Instead of having one test case that checks two possible things, make a dedicated test case for each possible input. That's what unit-testing is all about.

    my $error_direct = ASC::Builder::Error->new('foo');
    is $error_direct->message, 'foo', 'direct error message gets read correctly';
    
    my $error_indirect = ASC::Builder::Error->new( { message => 'bar' } );
    is $error_indirect->message, 'bar', 'indirect error message gets read correctly';
    

    The above code will give you two test cases. One for a direct error string, and another one for an indirect hash.

    ok 1 - direct error message gets read correctly
    ok 2 - indirect error message gets read correctly
    1..2
    

    Don't waste time

    At the same time, this also addresses another issue with your approach. In unit tests, you want to test the smallest possible unit. Don't tie them to your other business logic or your business production data.

    Your ASC::Builder::Error class doesn't care about the type of error, so don't over-complicate by loading something additonal to give you the exact same error messages you have in real life. Just use simple things that are enough to prove stuff works.

    The simpler your unit tests are, the easier it is to maintain them, and the easier it is to add more once you have more cases.