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
