Bugzilla – Attachment 19178 Details for
Bug 10386
UT: VirtualShelves.t needs some more adjustments
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10386: VirtualShelves.t needs some more adjustments
Bug-10386-VirtualShelvest-needs-some-more-adjustme.patch (text/plain), 10.53 KB, created by
Kyle M Hall (khall)
on 2013-06-21 12:39:19 UTC
(
hide
)
Description:
Bug 10386: VirtualShelves.t needs some more adjustments
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2013-06-21 12:39:19 UTC
Size:
10.53 KB
patch
obsolete
>From ed9ee7b37003bc28e96f9a27daa610611d5ecf8e Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Mon, 3 Jun 2013 15:27:50 +0200 >Subject: [PATCH] Bug 10386: VirtualShelves.t needs some more adjustments > >Most important: Does no longer delete all shelves! >Checks if there are ten borrowers for testing. But even works without them :) >When creating or modifying lists, takes name clashes into consideration. > >Small change to _CheckShelfName in VirtualShelves module. Making it possible to >check a name for a list whose owner has been set to NULL. Note that a test >like field=? with undef for placeholder will not work in MySql. > >Test plan: >How do you test a test? Well, you could run it on various databases.. >But for real hacking, you could also add some debug lines. >I tested this by forcing 10 undefs in @borrowernumbers. >And by overwriting the return value of randomname with an existing name. > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/VirtualShelves.pm | 13 +++- > t/db_dependent/VirtualShelves.t | 168 +++++++++++++++++++++++++-------------- > 2 files changed, 118 insertions(+), 63 deletions(-) > >diff --git a/C4/VirtualShelves.pm b/C4/VirtualShelves.pm >index ed489f2..38fa0e8 100644 >--- a/C4/VirtualShelves.pm >+++ b/C4/VirtualShelves.pm >@@ -681,19 +681,26 @@ sub _CheckShelfName { > my ($name, $cat, $owner, $number)= @_; > > my $dbh = C4::Context->dbh; >+ my @pars; > my $query = qq( > SELECT DISTINCT shelfnumber > FROM virtualshelves > LEFT JOIN virtualshelfshares sh USING (shelfnumber) > WHERE shelfname=? AND shelfnumber<>?); >- if($cat==1) { >+ if($cat==1 && defined($owner)) { > $query.= ' AND (sh.borrowernumber=? OR owner=?) AND category=1'; >+ @pars=($name, $number, $owner, $owner); > } >- else { >+ elsif($cat==1 && !defined($owner)) { #owner is null (exceptional) >+ $query.= ' AND owner IS NULL AND category=1'; >+ @pars=($name, $number); >+ } >+ else { #public list > $query.= ' AND category=2'; >+ @pars=($name, $number); > } > my $sth = $dbh->prepare($query); >- $sth->execute($cat==1? ($name, $number, $owner, $owner): ($name, $number)); >+ $sth->execute(@pars); > return $sth->rows>0? 0: 1; > } > >diff --git a/t/db_dependent/VirtualShelves.t b/t/db_dependent/VirtualShelves.t >index 323cc2c..563eeca 100755 >--- a/t/db_dependent/VirtualShelves.t >+++ b/t/db_dependent/VirtualShelves.t >@@ -1,12 +1,11 @@ > #!/usr/bin/perl > >-# > # This file is a test script for C4::VirtualShelves.pm > # Author : Antoine Farnault, antoine@koha-fr.org >-# >+# Larger modifications by Jonathan Druart and Marcel de Rooy > > use Modern::Perl; >-use Test::More tests => 82; >+use Test::More tests => 81; > use MARC::Record; > > use C4::Biblio qw( AddBiblio DelBiblio ); >@@ -14,16 +13,16 @@ use C4::Context; > > # Getting some borrowers from database. > my $dbh = C4::Context->dbh; >-my $query = q{ >- SELECT borrowernumber >- FROM borrowers >- LIMIT 10 >-}; >-my $sth = $dbh->prepare($query); >-$sth->execute; >+my $query = q{SELECT borrowernumber FROM borrowers LIMIT 10}; >+my $borr_ref=$dbh->selectall_arrayref($query); >+if(@$borr_ref==0) { #no borrowers? should not occur of course >+ $borr_ref->[0][0]=undef; >+ #but even then, we can run this robust test :) >+} > my @borrowers; >-while(my $borrower = $sth->fetchrow){ >- push @borrowers, $borrower; >+foreach(1..10) { >+ my $t= $_> @$borr_ref ? int(rand()*@$borr_ref): $_-1; #repeat if not enough >+ push @borrowers, $borr_ref->[$t][0]; > } > > # Creating some biblios >@@ -34,20 +33,6 @@ foreach(0..9) { > push @biblionumbers, $biblionumber; > } > >-# FIXME Why are you deleting my shelves? See bug 10386. >-my $delete_virtualshelf = q{ >- DELETE FROM virtualshelves WHERE 1 >-}; >-my $delete_virtualshelfcontent = q{ >- DELETE FROM virtualshelfcontents WHERE 1 >-}; >- >-$sth = $dbh->prepare($delete_virtualshelf); >-$sth->execute; >-$sth = $dbh->prepare($delete_virtualshelfcontent); >-$sth->execute; >-# --- >- > #----------------------------------------------------------------------# > # > # TESTS START HERE >@@ -59,33 +44,61 @@ use_ok('C4::VirtualShelves'); > #-----------------------TEST AddShelf function------------------------# > # usage : $shelfnumber = &AddShelf( $shelfname, $owner, $category); > >-# creating 10 good shelves. >+# creating shelves (could be <10 when names are not unique) > my @shelves; >-for(my $i=0; $i<10;$i++){ >- my $ShelfNumber = AddShelf( >- {shelfname=>"Shelf_".$i, category=>int(rand(2))+1 }, $borrowers[$i] ); >- die "test Not ok, remove some shelves before" if ($ShelfNumber == -1); >- ok($ShelfNumber > -1, "created shelf"); # Shelf creation successful; >- push @shelves, $ShelfNumber if $ShelfNumber > -1; >+for(my $i=0; $i<10;$i++) { >+ my $name= randomname(); >+ my $catg= int(rand(2))+1; >+ my $ShelfNumber= AddShelf( >+ { >+ shelfname => $name, >+ category => $catg, >+ }, >+ $borrowers[$i]); >+ >+ if($ShelfNumber>-1) { >+ ok($ShelfNumber > -1, "created shelf"); # Shelf creation successful; >+ } >+ else { >+ my $t= C4::VirtualShelves::_CheckShelfName( >+ $name, $catg, $borrowers[$i], 0); >+ ok($t==0, "Name clash expected on shelf creation"); >+ } >+ push @shelves, { >+ number => $ShelfNumber, >+ name => $name, >+ catg => $catg, >+ owner => $borrowers[$i], >+ }; #also push the errors > } > >-ok(10 == scalar @shelves, 'created 10 lists'); # 10 shelves in @shelves; >- >-# try to create some shelf which already exists. >+# try to create shelves with duplicate names > for(my $i=0;$i<10;$i++){ >- my @shlf=GetShelf($shelves[$i]); >- >- # FIXME This test still needs some attention >- # A shelf name is not per se unique ! See report 10386 >- #temporary hack: Original test only for public list >- if( $shlf[3]==2 ) { >- my $badNumShelf= AddShelf( >- {shelfname=>"Shelf_".$i, category => 2}, $borrowers[$i]); >+ if($shelves[$i]->{number}<0) { >+ ok(1, 'skip duplicate test for earlier name clash'); >+ next; >+ } >+ my @shlf=GetShelf($shelves[$i]->{number}); #number, name, owner, catg, ... >+ >+ # A shelf name is not per se unique! >+ if( $shlf[3]==2 ) { #public list: try to create with same name >+ my $badNumShelf= AddShelf( { >+ shelfname=> $shelves[$i]->{name}, >+ category => 2 >+ }, $borrowers[$i]); > ok(-1==$badNumShelf, 'do not create public lists with duplicate names'); > #AddShelf returns -1 if name already exist. >+ DelShelf($badNumShelf) if $badNumShelf>-1; #delete if went wrong.. > } >- else { >- ok(1==1, "This trivial test cannot fail :)"); #leave count intact >+ else { #private list, try to add another one for SAME user (owner) >+ my $badNumShelf= defined($shlf[2])? AddShelf( >+ { >+ shelfname=> $shelves[$i]->{name}, >+ category => 1, >+ }, >+ $shlf[2]): -1; >+ ok(-1==$badNumShelf, 'do not create private lists with duplicate name for same user'); >+ DelShelf($badNumShelf) if $badNumShelf>-1; #delete if went wrong.. > } > } > >@@ -97,10 +110,17 @@ for(my $i=0;$i<10;$i++){ > my %used = (); > for(my $i=0; $i<10;$i++){ > my $bib = $biblionumbers[int(rand(9))]; >- my $shelfnumber = $shelves[int(rand(9))]; >+ my $shelfnumber = $shelves[int(rand(9))]->{number}; >+ if($shelfnumber<0) { >+ ok(1, 'skip add to list-test for shelf -1'); >+ ok(1, 'skip counting list entries for shelf -1'); >+ next; >+ } > > my $key = "$bib\t$shelfnumber"; > my $should_fail = exists($used{$key}) ? 1 : 0; >+ #FIXME We assume here that we have permission to add.. >+ #The different permissions could be tested too. > > my ($biblistBefore,$countbefore) = GetShelfContents($shelfnumber); > my $status = AddToShelf($bib,$shelfnumber,$borrowers[$i]); >@@ -115,11 +135,10 @@ for(my $i=0; $i<10;$i++){ > if (defined $status) { > ok($countbefore == $countafter - 1, 'added bib to list'); # the bib has been successfuly added. > } else { >- ok($countbefore == $countafter, 'did not add duplicate bib to list'); # the bib has been successfuly added. >+ ok($countbefore == $countafter, 'did not add duplicate bib to list'); > } > > $used{$key}++; >- > } > > #-----------------------TEST ModShelf & GetShelf functions------------------------# >@@ -128,26 +147,55 @@ for(my $i=0; $i<10;$i++){ > > for(my $i=0; $i<10;$i++){ > my $rand = int(rand(9)); >- my $numA = $shelves[$rand]; >- my $shelf = { shelfname => "NewName_".$rand, >- category => int(rand(2))+1 }; >- >- ModShelf($numA,$shelf); >- my ($numB,$nameB,$ownerB,$categoryB) = GetShelf($numA); >- >- ok($numA == $numB, 'modified shelf'); >- ok($shelf->{shelfname} eq $nameB, '... and name change took'); >- ok($shelf->{category} eq $categoryB, '... and category change took'); >+ my $numA = $shelves[$rand]->{number}; >+ if($numA<0) { >+ ok(1, 'Skip ModShelf test for shelf -1'); >+ ok(1, 'Skip ModShelf test for shelf -1'); >+ ok(1, 'Skip ModShelf test for shelf -1'); >+ next; >+ } >+ my $newname= randomname(); >+ my $shelf = { >+ shelfname => $newname, >+ category => 3-$shelves[$rand]->{catg}, # tric: 1->2 and 2->1 >+ }; >+ #check name change (with category change) >+ if(C4::VirtualShelves::_CheckShelfName($newname,$shelf->{category}, >+ $shelves[$rand]->{owner}, $numA)) { >+ ModShelf($numA,$shelf); >+ my ($numB,$nameB,$ownerB,$categoryB) = GetShelf($numA); >+ ok($numA == $numB, 'modified shelf'); >+ ok($shelf->{shelfname} eq $nameB, '... and name change took'); >+ ok($shelf->{category} eq $categoryB, '... and category change took'); >+ } >+ else { >+ ok(1, "No ModShelf for $newname") for 1..3; >+ } > } > > #-----------------------TEST DelShelf & DelFromShelf functions------------------------# > # usage : ($status) = &DelShelf($shelfnumber); > > for(my $i=0; $i<10;$i++){ >- my $shelfnumber = $shelves[$i]; >+ my $shelfnumber = $shelves[$i]->{number}; >+ if($shelfnumber<0) { >+ ok(1, 'Skip DelShelf for shelf -1'); >+ next; >+ } > my $status = DelShelf($shelfnumber); > ok(1 == $status, "deleted shelf $shelfnumber and its contents"); > } > > #----------------------- CLEANUP ----------------------------------------------# >+ > DelBiblio($_) for @biblionumbers; >+ >+#----------------------- SOME SUBS --------------------------------------------# >+ >+sub randomname { >+ my $rv=''; >+ for(0..19) { >+ $rv.= ('a'..'z','A'..'Z',0..9) [int(rand()*62)]; >+ } >+ return $rv; >+} >-- >1.7.2.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 10386
:
18821
|
19178
|
19273