Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss
Re: CPANifying our test framework - or parts of it
2 views
Skip to first unread message
James E Keenan
Sep 10, 2016, 10:00:02 AM9/10/16
to per...@perl.org
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
> 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
Andy Lester
Sep 10, 2016, 3:15:01 PM9/10/16
to Sam Kington, Perl QA
> On Sep 9, 2016, at 8:34 PM, Sam Kington <s...@illuminated.co.uk> wrote:
>
> How should I best extract this functionality into a proper CPAN distribution (ideally using Test2)?
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.
}
}
structures => array(
hash => {
field structure_id => match qr{^ (?<structure_id> STRUCT \d+ ) $}x;
field type => 'dolmen',
field material => 'concrete',
}
}
--
Andy Lester => www.petdance.com
David Cantrell
Sep 12, 2016, 1:00:03 PM9/12/16
to per...@perl.org
On Sat, Sep 10, 2016 at 03:34:02AM +0200, Sam Kington wrote:
> (A) To say, flexibly, ???I want a data structure that looks like this???, and to be able to say (1) this field must look exactly like this, (2) this field should look like this, and remember it, (3) this field should exist, but I don???t care what its value is, and (4) there might be additional fields but I don???t care about them for the purpose of this test.
Investigate Params::Validate (which you're already using elsewhere in
your code, although you're not using all of its functionality). It's pretty
powerful.
For checking deeply nested data structures in XML I like XPath.
Data::DPath appears to allow similar syntax for inspecting perl data
structures.
--
David Cantrell | London Perl Mongers Deputy Chief Heretic
Just because it is possible to do this sort of thing
in the English language doesn't mean it should be done
> (A) To say, flexibly, ???I want a data structure that looks like this???, and to be able to say (1) this field must look exactly like this, (2) this field should look like this, and remember it, (3) this field should exist, but I don???t care what its value is, and (4) there might be additional fields but I don???t care about them for the purpose of this test.
Investigate Params::Validate (which you're already using elsewhere in
your code, although you're not using all of its functionality). It's pretty
powerful.
For checking deeply nested data structures in XML I like XPath.
Data::DPath appears to allow similar syntax for inspecting perl data
structures.
--
David Cantrell | London Perl Mongers Deputy Chief Heretic
Just because it is possible to do this sort of thing
in the English language doesn't mean it should be done
0 new messages