t/db_dependent/HoldsQueue.t depends on the Jenkins database. We should create the data we want and delete them at the end.
Created attachment 18388 [details] [review] WIP: HoldsQueue.t needs to create its own data
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?
I create a new database containing sample data with script submitted on bug 10337.
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.
(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.
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 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.
Created attachment 19167 [details] [review] WIP: HoldsQueue.t needs to create its own data
Thanks Galen! Unfortunately, 2 tests still don't pass.
(In reply to comment #9) > Unfortunately, 2 tests still don't pass. Bug 10495 may be relevant.
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.
Created attachment 19177 [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.
(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
Created attachment 19179 [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.
(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?
Created attachment 19191 [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>
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>
Pushed to master. Thanks, Jonathan!
This patch has been pushed to 3.12.x, will be in 3.12.2. Thanks Jonathan!
Pushed to 3.10.x, will be in 3.10.8