Bug 10336

Summary: UT: HoldsQueue.t needs to create its own data
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Test SuiteAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P5 - low CC: bgkriegel, gmcharlt, kyle.m.hall, kyle, tomascohen
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:
Bug Depends on:    
Bug Blocks: 10273, 10495    
Attachments: WIP: HoldsQueue.t needs to create its own data
WIP: HoldsQueue.t needs to create its own data
Bug 10336: HoldsQueue.t needs to create its own data
Bug 10336: HoldsQueue.t needs to create its own data
Bug 10336: HoldsQueue.t needs to create its own data
Bug 10336: HoldsQueue.t needs to create its own data

Description Jonathan Druart 2013-05-24 14:23:36 UTC
t/db_dependent/HoldsQueue.t depends on the Jenkins database.
We should create the data we want and delete them at the end.
Comment 1 Jonathan Druart 2013-05-24 14:31:26 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-05-24 14:33:43 UTC
Hi Kyle,

I tried to pass HoldsQueue unit tests but I am not able to do :-/
The idea is to create all data the tests need before launching them in order not to be dependent on the Jenkins database.

Could you take a look at it please?
Comment 3 Jonathan Druart 2013-05-24 14:51:54 UTC
I create a new database containing sample data with script submitted on bug 10337.
Comment 4 Kyle M Hall 2013-06-03 12:10:13 UTC
Looks like this needs updated a bit.

 perl t/db_dependent/HoldsQueue.t
1..18
ok 1 - use C4::Reserves;
ok 2 - use C4::HoldsQueue;
Type of arg 1 to keys must be hash (not subroutine entry) at t/db_dependent/HoldsQueue.t line 42, near "GetBranches;"
Execution of t/db_dependent/HoldsQueue.t aborted due to compilation errors.
# Looks like you planned 18 tests but ran 2.
# Looks like your test exited with 255 just after 2.
Comment 5 Jonathan Druart 2013-06-04 12:44:48 UTC
(In reply to comment #4)
> Looks like this needs updated a bit.

Kyle, did you seen my comment 2? :)
Do you plan to work on it? That would be really good if this file passes all the tests. I managed to pass all tests from all others files, but not these ones.
Comment 6 Kyle M Hall 2013-06-04 13:21:56 UTC
I read it, but I didn't fully understand it until today. I should have noticed the WIP in the title. I hadn't planned on working on this. If I can get some free time I'd be more than happy to assist! However, I cannot say when that may be.

(In reply to comment #5)
> (In reply to comment #4)
> > Looks like this needs updated a bit.
> 
> Kyle, did you seen my comment 2? :)
> Do you plan to work on it? That would be really good if this file passes all
> the tests. I managed to pass all tests from all others files, but not these
> ones.
Comment 7 Galen Charlton 2013-06-19 17:23:45 UTC
Comment on attachment 18388 [details] [review]
WIP: HoldsQueue.t needs to create its own data

Review of attachment 18388 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/HoldsQueue.t
@@ +35,5 @@
> +    branchcode => 'CPL',
> +);
> +
> +my $borrowernumber = AddMember(%data);
> +my $borrower = GetMember( borrowernumber => $borrowernumber );

Looks good -- this also removes the requirement that there be at least one item on loan in the database, which isn't relevant to the functionality being tested.

@@ +51,2 @@
>  $dbh->{AutoCommit} = 0;
>  $dbh->{RaiseError} = 1;

This section should be moved to before the call to AddMember.  That way, we don't need to call DelMember to clean up; the rollback will take care of that.

@@ +153,5 @@
>  $dbh->rollback;
>  
> +END {
> +    C4::Members::DelMember( $borrowernumber );
> +};

Not needed -- see above comment.
Comment 8 Jonathan Druart 2013-06-20 07:28:22 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2013-06-20 07:28:41 UTC
Thanks Galen!
Unfortunately, 2 tests still don't pass.
Comment 10 Galen Charlton 2013-06-20 13:59:49 UTC
(In reply to comment #9)
> Unfortunately, 2 tests still don't pass.

Bug 10495 may be relevant.
Comment 11 Galen Charlton 2013-06-20 15:49:00 UTC
Please see my comment in bug 10495.  If you attach a non-WIP version of your patch, I'm ready to test it and sign off on it.
Comment 12 Jonathan Druart 2013-06-21 07:22:14 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2013-06-21 12:37:44 UTC
(In reply to comment #12)
> Created attachment 19177 [details] [review] [review]
> Bug 10336: HoldsQueue.t needs to create its own data
> 
> After applying this patch and the patch submitted in bug 10495, you can
> run prove t/db_dependent/HoldsQueue.t and admire that all tests pass.
> 
> This patch creates a borrower and uses Koha routines instead of directly
> request the database.

Then I apply both patches and run HoldsQueue.t, I get the following error:

 perl t/db_dependent/HoldsQueue.t
1..18
ok 1 - use C4::Reserves;
ok 2 - use C4::HoldsQueue;
Type of arg 1 to keys must be hash (not subroutine entry) at t/db_dependent/HoldsQueue.t line 47, near "GetBranches;"
Execution of t/db_dependent/HoldsQueue.t aborted due to compilation errors.
# Looks like you planned 18 tests but ran 2.
# Looks like your test exited with 255 just after 2.

And this:

prove t/db_dependent/HoldsQueue.t
t/db_dependent/HoldsQueue.t .. 1/18 Type of arg 1 to keys must be hash (not subroutine entry) at t/db_dependent/HoldsQueue.t line 47, near "GetBranches;"
Execution of t/db_dependent/HoldsQueue.t aborted due to compilation errors.
# Looks like you planned 18 tests but ran 2.
# Looks like your test exited with 255 just after 2.
t/db_dependent/HoldsQueue.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 16/18 subtests 

Test Summary Report
-------------------
t/db_dependent/HoldsQueue.t (Wstat: 65280 Tests: 2 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 18 tests but ran 2.
Files=1, Tests=2,  1 wallclock secs ( 0.02 usr  0.01 sys +  0.60 cusr  0.05 csys =  0.68 CPU)
Result: FAIL
Comment 14 Jonathan Druart 2013-06-21 12:44:45 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2013-06-21 12:46:29 UTC
(In reply to comment #13)
> Type of arg 1 to keys must be hash (not subroutine entry) at
> t/db_dependent/HoldsQueue.t line 47, near "GetBranches;"

It's certainly caused by the perl version (I am using v.5.14).
Could you try again with the last patch please?
Comment 16 Kyle M Hall 2013-06-21 13:46:53 UTC Comment hidden (obsolete)
Comment 17 Chris Cormack 2013-06-27 08:08:08 UTC
Created attachment 19256 [details] [review]
Bug 10336: HoldsQueue.t needs to create its own data

After applying this patch and the patch submitted in bug 10495, you can
run prove t/db_dependent/HoldsQueue.t and admire that all tests pass.

This patch creates a borrower and uses Koha routines instead of directly
request the database.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 18 Galen Charlton 2013-06-27 15:27:24 UTC
Pushed to master.  Thanks, Jonathan!
Comment 19 Tomás Cohen Arazi 2013-07-15 18:08:32 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks Jonathan!
Comment 20 Bernardo Gonzalez Kriegel 2013-07-16 12:02:51 UTC
Pushed to 3.10.x, will be in 3.10.8