develooper Front page | perl.qa | Postings from September 2016

Re: CPANifying our test framework - or parts of it

Thread Previous | Thread Next
From:
James E Keenan
Date:
September 10, 2016 13:43
Subject:
Re: CPANifying our test framework - or parts of it
Message ID:
20160910134250.6869.qmail@lists-nntp.develooper.com
On 09/09/2016 09:34 PM, Sam Kington wrote:
> Hi,
>
> At $WORK we have an extensive test suite that we’ve built up over the years, with loads of convenience methods for calling e.g. Dancer endpoints and checking returned data structures against what we expect. One of our dev team recently left for pastures new, and would like to carry on using these tools. How should I best extract this functionality into a proper CPAN distribution (ideally using Test2)?
>

Let me state my overall impression at the output.  It's apparent that 
many (hundreds?) of hours of work have been put into the development of 
this testing apparatus, but if it were put on CPAN I doubt I would use 
it.  It appears to be too heavily designed toward your specific use 
cases and, in certain respects, is designed in a way that does not sit 
well with me.

Comments inline.

> At its most elaborate, our current test code lets you say e.g.
>
> # Assume $self->state('location_id') has been set previously
> $self->test(

Of what class is $self an instance?  The class of the object being 
tested (as inferred from the existence of a 'state' method)?  Or a 
specialized testing class (as inferred from the existence of a 'test' 
method)?

It seems that you are mixing both real object data with testing data in 
the same object.  Speaking for myself, I don't think that's a clean design.

>     title => 'Create a properly megalithic structure',
>     call  => [
>         POST => '/location/:location_id/build',
>         {
>             author     => 'Wally Wallington',
>             structures => [
>                 {
>                     type     => 'dolmen',
>                     material => 'concrete',
>                 }
>             ]
>         }
>     ],
>     expect => {
>         http_code => HTTP_CREATED,
>         sql_count => 12,
>         data      => {
>             location_id => $self->state('location_id'),
>             build_id    => qr{^ (?<build_id> BUILD \d+ ) $}x,
>             built       => $self->true,
>             author     => $self->meh,    # It's OK if they call him Idiot.
>             structures => [
>                 {
>                     _hashref_contains => {
>                         structure_id => qr{^ (?<structure_id> STRUCT \d+ ) $}x,
>                         type         => 'dolmen',
>                         material     => 'concrete',
>
>                         # There's probably stuff about where the dolmen
>                         # was erected but we ignore that for the purpose
>                         # of this test.
>                     }
>                 }
>             ]
>         }
>     }
> );
>

'test' is too general a name for this method, as the method is highly 
engineered for your specific needs.  But that's a quibble.  More 
important is the fact that you must have a lot of code under the hood to 
generate the hash reference which is the value of the 'expect' key. 
That code is inquiring about many different attributes of the object and 
it has to get *every one* of those attributes right for the unit test to 
pass.  You're making *one, big assertion* about the state of the object 
-- and you're probably making a strong assumption about the internal 
structure of the object.

An alternative approach would be to create the object, then write method 
calls which focus on individual attributes expected within that object:

#####
my $obj = $self->call( [
          POST => '/location/:location_id/build',
          {
              author     => 'Wally Wallington',
              structures => [
                  {
                      type     => 'dolmen',
                      material => 'concrete',
                  }
              ]
          }
      ]
);
is($obj->http_code, HTTP_CREATED, "Got expected HTTP response code");
ok($obj->built, "'built' set to true value");
ok(length($obj->author),
   "I don't care what was set for 'author' as long as it's a 
non-zero-length string");
#####

One more specific objection:

#####
 >     expect => {
...
 >             build_id    => qr{^ (?<build_id> BUILD \d+ ) $}x,
...
 >     }
...
 > # $self->state('build_id') got set by the named capture in the regex 
above.
#####

Is the 'build_id' something set by the process of creation of a new 
object and intrinsically part of that new object?  Or is it an artifact 
of testing that object?

And if it is (as I suspect) generated in the creation of a new object, 
is its value predictable in advance?  If not predictable in advance -- 
if, say, it's partly composed of an epoch timestamp --  then you can't 
make any assertion about what its value ought to be.  That, in turn, 
means that it has no place in the 'expect' part of a unit test.

I would raise the same objection with respect to 'structure_id'.  I'd be 
much more inclined to make method calls on the new object and store the 
return values in variables for later use:

#####
my $build_id = $obj->build_id();
my $structure_id = $obj->build_id();
my $newobj = $self->call( [
          PATCH => 
"/location/$location_id/build/$build_id/structure/$structure_id",
          { material => 'gold' }
      ]
);
#####

Now, I concede that some of my objections are a matter of taste. 
TIMTOWTDI.  But I've written a lot of testing code over the years -- 
probably more testing code than "production" code -- and my gut feeling 
is that your testing apparatus is over-engineered for general use -- and 
perhaps even over-engineered for your own use.

Thank you very much.
Jim Keenan

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About