[jifty-devel] I would like some feedback on my changes

[Jesse Vincent]

On Tue, Nov 13, 2007 at 10:26:18AM -0500, Jason May wrote:
> This is my first time applying test-driven development to anything,
> and since I am committing this code, I would like some feedback first.
> Thanks.

Ok. you probably want to do this in Jifty::DBI's tests rather than
jifty's

> 
> ===================================================================
> --- t/TestApp/t/19-group_by.t	(revision 0)
> +++ t/TestApp/t/19-group_by.t	(revision 0)
> @@ -0,0 +1,39 @@
> +#!/usr/bin/env perl
> +use warnings;
> +use strict;
> +
> +=head1 DESCRIPTION
> +
> +A test harness for User model.
> +
> +=cut
> +
> +use lib 't/lib';
> +use Jifty::SubTest;
> +
> +use Jifty::Test skip_all => 'Not fully implemented yet';
> +Jifty::Test->web;
> +
> +# Make sure we can load the model collection
> +use_ok('TestApp::Model::UserCollection');
> +
> +# Grab a system user
> +my $system_user = TestApp::CurrentUser->superuser;
> +
> +my $o = TestApp::Model::User->new(current_user => $system_user);
> +$o->create(name => $$,		score => $$,		email => $$);
> +$o->create(name => $$,		score => $$+50,		email => $$);
> +$o->create(name => $$+1,	score => $$+100,	email => $$);
> +$o->create(name => $$+1,	score => $$+200,	email => $$);
> 

A couple things here:

1) Don't create multiple records with the same object. you want
$o,$p,$q,$r all as user objects

2) Test those create returns. (And do you mean to create four users with
two names and one email address?)

+
> +my $c = TestApp::Model::UserCollection->new(current_user => $system_user);
> +$c->unlimit;

You shouldn't need to call unlimit

> +$c->column(column 	=> 'name');
> +$c->column(column 	=> 'high_score', function => 'max(score)');
> +$c->group_by(column	=> 'name');
> +
> +ok($c->build_select_query() =~ /group by/i,
> +		'GROUP BY clause successfully placed in the query');
> +
> +my $f = $c->first;
> +ok($f, 'GROUP BY query syntax is functional');


You need two more tests here:

checking $f->high_score and checking that $f->id is the same as  $r->id

Best,
Jesse


> Index: t/TestApp/lib/TestApp/Model/User.pm
> ===================================================================
> --- t/TestApp/lib/TestApp/Model/User.pm	(revision 4422)
> +++ t/TestApp/lib/TestApp/Model/User.pm	(working copy)
> @@ -16,6 +16,8 @@
>  column 'email' =>
>    type is 'text',
>    is mandatory;
> +column 'score' =>
> +  type is 'integer';
>  column 'really_tasty' =>
>    type is 'boolean',
>    is immutable,
> _______________________________________________
> jifty-devel mailing list
> jifty-devel at lists.jifty.org
> http://lists.jifty.org/cgi-bin/mailman/listinfo/jifty-devel
> 

--