Bug 10441

Summary: UT: Testing AddShare in VirtualShelves.t
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Test SuiteAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED MOVED QA Contact:
Severity: normal    
Priority: P5 - low CC: jonathan.druart, m.de.rooy, srdjan
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 10441: Testing AddShare in VirtualShelves.t
Bug 10441: Testing AddShare in VirtualShelves.t
[SIGNED-OFF] Bug 10441: Testing AddShare in VirtualShelves.t
Bug 10441: QA Followup for Virtualshelves.t
Bug 10441: UT: Create patrons for Virtualshelves.t

Description Marcel de Rooy 2013-06-10 12:58:15 UTC
Builds upon bug 10386 and bug 9032.
Adds a unit test for routine AddShare.
Comment 1 Marcel de Rooy 2013-06-10 13:02:38 UTC Comment hidden (obsolete)
Comment 2 Srdjan Jankovic 2013-06-11 03:04:13 UTC
I'm not disputing the test itself, but wouldn't it be nice if AddShare returned something to indicate sucsess/failure?
Comment 3 Marcel de Rooy 2013-06-11 14:51:23 UTC
(In reply to comment #2)
> I'm not disputing the test itself, but wouldn't it be nice if AddShare
> returned something to indicate sucsess/failure?

Thanks for testing.
First of all, you are right of course.
By far, we are not testing the success of mysql insert statements in Koha. In some cases it is done (saw one when e.g. adding a biblio); note that AddShare is not the most critical operation :) AddShare could however simply check DBI's err and return a boolean.
Will leave the two reports as they are now for the moment, but will later handle this in a followup.
Comment 4 Marcel de Rooy 2013-07-17 13:58:47 UTC
See also report 10499. This patch removed the DelShelf code. Will include it again in rebasing this patch.
Comment 5 Marcel de Rooy 2013-07-17 14:06:12 UTC Comment hidden (obsolete)
Comment 6 Srdjan Jankovic 2013-07-17 23:52:46 UTC Comment hidden (obsolete)
Comment 7 Srdjan Jankovic 2013-07-17 23:53:51 UTC
Just an idea, maybe use SKIP block rather than if().
Comment 8 Marcel de Rooy 2013-07-18 06:57:32 UTC
(In reply to Srdjan Jankovic from comment #7)
> Just an idea, maybe use SKIP block rather than if().

Thanks for testing, Srdjan.
Yes, could have used skip here too. Can imagine cases where skip would really be a better choice. In this case fairly arbitrary..
Comment 9 Jonathan Druart 2013-07-18 14:34:00 UTC
Quick review:

Please prefer "is" instead of "ok":

  ok(1 == $status, "deleted shelf $shelfnumber and its contents");
  is( $status, 1, "deleted shelf $shelfnumber and its contents");

and
  for my $i ( 0 .. 9 ) {
instead of
  for(my $i=0; $i<10;$i++){
Comment 10 Marcel de Rooy 2013-07-25 07:43:29 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2013-08-20 14:33:00 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2013-08-27 19:58:28 UTC
Patch this depends on is currently 'failed qa' - using blocked to indicate that this can't be QA'd right now.
Comment 13 Marcel de Rooy 2014-03-01 10:02:58 UTC
I will move this code to bug 9032.
Close this report now.