We are checking if we have at least 1 item type not for loan: 63 my @item_types = C4::ItemType->all; 64 my $itemtype = grep { $_->{notforloan} == 1 } @item_types 65 or BAIL_OUT("No adequate itemtype"); Then we use the $itemtype variable (which contains the number of item types not for loan): 92 $dbh->do("INSERT INTO biblioitems (biblionumber, marcxml, itemtype) 93 VALUES ($biblionumber, '', '$itemtype')"); There is obviously something wrong here. The code should be 64 my @not_for_loan = grep { $_->{notforloan} == 1 } @item_types 65 or BAIL_OUT("No adequate itemtype"); 66 my $itemtype = $not_for_loan[0]->{itemtype}; But then some tests don't pass: # Looks like you failed 8 tests of 17 run.
Kyle, could you have a look please? I don't manage to get what is wrong here.
FYI, this is still on my radar, I just haven't gotten to it yet.
The "itemtype" from line 64 is being inserted into a biblioitems row at line , which does not have a foreign key to itemtypes, so it succeeds. The itemtype is re-used at line 98 for an item. It appears the test actually requires the item to be *for* loan, rather than not for loan. Since the itemtype doesn't really exists, is seems to fail as a for loan itemtype, even though it seems like we should get an execution error instead. Then, at line 181 the itemtype is replace with an arbitrary itemtype selected from the database, which is probably just a lucky grab since the default data would give a for loan itemtype first.
Also, I kind of expected this tests to fail when I set item level itypes to biblio, but it still continued to succeed. It seems that the selection of a not for loan itemtype is just cruft at this point.
Created attachment 46911 [details] [review] Bug 15391: Fix HoldsQueue.t tests Prior to this patch, in HoldsQueue.t: 63 my @item_types = C4::ItemType->all; 64 my $itemtype = grep { $_->{notforloan} == 1 } @item_types 65 or BAIL_OUT("No adequate itemtype"); Then we use the $itemtype variable (which contains the number of item types not for loan): 92 $dbh->do("INSERT INTO biblioitems (biblionumber, marcxml, itemtype) 93 VALUES ($biblionumber, '', '$itemtype')"); There is obviously something wrong here. The code should be 64 my @not_for_loan = grep { $_->{notforloan} == 1 } @item_types 65 or BAIL_OUT("No adequate itemtype"); 66 my $itemtype = $not_for_loan[0]->{itemtype}; But then some tests don't pass: Actually the problem comes from: commit bfbc646fdd9ca4b90a0bc2751d0faa95d9e93ba1 Bug 10336: HoldsQueue.t needs to create its own data -my $itemtype = $dbh->selectrow_array("SELECT min(itemtype) FROM itemtypes WHERE notforloan = 0") +my @item_types = C4::ItemType->all; +my $itemtype = grep { $_->{notforloan} == 1 } @item_types The line should have been: my $itemtype = grep { $_->{notforloan} == 0 } @item_types Test plan: Confirm that the tests still pass after this patch applied.
t/db_dependent/HoldsQueue.t .. 3/23 # Failed test 'Bug 15062 - Holds queue with Transport Cost Matrix will transfer item even if transfers disabled' # at t/db_dependent/HoldsQueue.t line 413. # got: '1' # expected: '0' # Looks like you failed 1 test of 23. t/db_dependent/HoldsQueue.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/23 subtests Test Summary Report ------------------- t/db_dependent/HoldsQueue.t (Wstat: 256 Tests: 23 Failed: 1) Failed test: 23 Non-zero exit status: 1 Files=1, Tests=23, 1 wallclock secs ( 0.02 usr 0.01 sys + 1.39 cusr 0.08 csys = 1.50 CPU) Result: FAIL
Aleisha, could you confirm the tests pass before the patch?
Created attachment 46990 [details] [review] Bug 15391: Fix HoldsQueue.t tests Prior to this patch, in HoldsQueue.t: 63 my @item_types = C4::ItemType->all; 64 my $itemtype = grep { $_->{notforloan} == 1 } @item_types 65 or BAIL_OUT("No adequate itemtype"); Then we use the $itemtype variable (which contains the number of item types not for loan): 92 $dbh->do("INSERT INTO biblioitems (biblionumber, marcxml, itemtype) 93 VALUES ($biblionumber, '', '$itemtype')"); There is obviously something wrong here. The code should be 64 my @not_for_loan = grep { $_->{notforloan} == 1 } @item_types 65 or BAIL_OUT("No adequate itemtype"); 66 my $itemtype = $not_for_loan[0]->{itemtype}; But then some tests don't pass: Actually the problem comes from: commit bfbc646fdd9ca4b90a0bc2751d0faa95d9e93ba1 Bug 10336: HoldsQueue.t needs to create its own data -my $itemtype = $dbh->selectrow_array("SELECT min(itemtype) FROM itemtypes WHERE notforloan = 0") +my @item_types = C4::ItemType->all; +my $itemtype = grep { $_->{notforloan} == 1 } @item_types The line should have been: my $itemtype = grep { $_->{notforloan} == 0 } @item_types Test plan: Confirm that the tests still pass after this patch applied. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> All tests pass pre and post patch
Created attachment 47061 [details] [review] [PASSED QA] Bug 15391: Fix HoldsQueue.t tests Prior to this patch, in HoldsQueue.t: 63 my @item_types = C4::ItemType->all; 64 my $itemtype = grep { $_->{notforloan} == 1 } @item_types 65 or BAIL_OUT("No adequate itemtype"); Then we use the $itemtype variable (which contains the number of item types not for loan): 92 $dbh->do("INSERT INTO biblioitems (biblionumber, marcxml, itemtype) 93 VALUES ($biblionumber, '', '$itemtype')"); There is obviously something wrong here. The code should be 64 my @not_for_loan = grep { $_->{notforloan} == 1 } @item_types 65 or BAIL_OUT("No adequate itemtype"); 66 my $itemtype = $not_for_loan[0]->{itemtype}; But then some tests don't pass: Actually the problem comes from: commit bfbc646fdd9ca4b90a0bc2751d0faa95d9e93ba1 Bug 10336: HoldsQueue.t needs to create its own data -my $itemtype = $dbh->selectrow_array("SELECT min(itemtype) FROM itemtypes WHERE notforloan = 0") +my @item_types = C4::ItemType->all; +my $itemtype = grep { $_->{notforloan} == 1 } @item_types The line should have been: my $itemtype = grep { $_->{notforloan} == 0 } @item_types Test plan: Confirm that the tests still pass after this patch applied. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> All tests pass pre and post patch Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to Master - Should be in the May 2016 release. Thanks
Patch pushed to 3.22.x, will be in 3.22.3
This patch has been pushed to 3.20.x, will be in 3.20.9.