Bug 10336 - UT: HoldsQueue.t needs to create its own data
Summary: UT: HoldsQueue.t needs to create its own data
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 10273 10495
  Show dependency treegraph
 
Reported: 2013-05-24 14:23 UTC by Jonathan Druart
Modified: 2014-12-07 20:02 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
WIP: HoldsQueue.t needs to create its own data (2.67 KB, patch)
2013-05-24 14:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
WIP: HoldsQueue.t needs to create its own data (2.74 KB, patch)
2013-06-20 07:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10336: HoldsQueue.t needs to create its own data (2.93 KB, patch)
2013-06-21 07:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10336: HoldsQueue.t needs to create its own data (2.97 KB, patch)
2013-06-21 12:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10336: HoldsQueue.t needs to create its own data (3.02 KB, patch)
2013-06-21 13:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10336: HoldsQueue.t needs to create its own data (3.08 KB, patch)
2013-06-27 08:08 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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