Bug 10495 - t/db_dependent/HoldsQueue.t can fail unnecessarily
Summary: t/db_dependent/HoldsQueue.t can fail unnecessarily
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Galen Charlton
QA Contact:
URL:
Keywords:
Depends on: 10336
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-19 17:27 UTC by Galen Charlton
Modified: 2014-12-07 20:02 UTC (History)
3 users (show)

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


Attachments
bug 10495: set precondition for HoldsQueue test (3.07 KB, patch)
2013-06-19 17:35 UTC, Galen Charlton
Details | Diff | Splinter Review
bug 10495: set precondition for HoldsQueue test (3.07 KB, patch)
2013-06-20 14:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
bug 10495: set precondition for HoldsQueue test (3.12 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 Galen Charlton 2013-06-19 17:27:30 UTC
The test case assumes that the AutomaticItemReturn system preference is off initially; if it starts off on, two of the tests fail:

not ok 4 - take from homebranch (don't use cost matrix) holding branch

#   Failed test 'take from homebranch (don't use cost matrix) holding branch'
#   at t/db_dependent/HoldsQueue.t line 162.
#          got: 'FFL'
#     expected: 'CPL'
[snip]
ok 5 - take from homebranch (use cost matrix) pick up branch
not ok 6 - take from homebranch (use cost matrix) holding branch

#   Failed test 'take from homebranch (use cost matrix) holding branch'
#   at t/db_dependent/HoldsQueue.t line 162.
#          got: 'UPL'
#     expected: 'CPL'
[snip]
Test Summary Report
-------------------
t/db_dependent/HoldsQueue.t (Wstat: 512 Tests: 18 Failed: 2)
  Failed tests:  4, 6
  Non-zero exit status: 2

The test case should explicitly turn off the AutomaticItemReturn syspref when it starts.
Comment 1 Galen Charlton 2013-06-19 17:35:55 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-06-20 14:29:06 UTC
Galen,
I got a bad news, with my DB tests, I have a bad news, with your patch I get:

  t/db_dependent/HoldsQueue.t .. 1/18 Use of uninitialized value $borrower_branchcode in string ne at t/db_dependent/HoldsQueue.t line 36.   
  Use of uninitialized value $borrower_branchcode in string ne at t/db_dependent/HoldsQueue.t line 36.
  [...]
  Use of uninitialized value $borrower_branchcode in string ne at t/db_dependent/HoldsQueue.t line 36.
  Use of uninitialized value $borrower_branchcode in join or string at t/db_dependent/HoldsQueue.t line 48.
  DBD::mysql::st execute failed: Column 'frombranch' cannot be null at t/db_dependent/HoldsQueue.t line 55.
  DBD::mysql::st execute failed: Column 'frombranch' cannot be null at t/db_dependent/HoldsQueue.t line 55.
  # 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,  0 wallclock secs ( 0.01 usr  0.01 sys +  0.43 cusr  0.02 csys =  0.47 CPU)
  Result: FAIL

But I have a good one, if I apply 10336 before yours, I get:
  t/db_dependent/HoldsQueue.t .. ok     
  All tests successful.
  Files=1, Tests=18,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.53 cusr  0.06 csys =  0.62 CPU)
  Result: PASS

So I will sign off this one! Thanks!
Comment 3 Jonathan Druart 2013-06-20 14:29:32 UTC Comment hidden (obsolete)
Comment 4 Galen Charlton 2013-06-20 15:47:57 UTC
(In reply to comment #2)
> Galen,
> I got a bad news, with my DB tests, I have a bad news, with your patch I get:

Yes, this relates to the comment I made in bug 10336 -- HoldsQueue.t currently assumes that there is at least one item out on loan; your patch removes that assumption.

Consequently, to whoever QAs this: this patch should be tested in conjunction with the patch for 10336.
Comment 5 Chris Cormack 2013-06-27 08:08:50 UTC
Created attachment 19257 [details] [review]
bug 10495: set precondition for HoldsQueue test

t/db_dependent/HoldsQueue.t assumed, but did not check,
that the AutomaticItemReturns system preference was off
at the beginning of the test un.  This patch makes sure
that that assumption is met.

To test:

[1] Make sure that at least one item is on loan (this is
    another assumption that the test case makes, one
    that should be corrected with the work proposed
    for bug 10336.
[2] Turn the AutomaticItemReturn system preference on.
[3] Run the test:

prove -v t/db_dependent/HoldsQueue.t

[4] Tests 4 and 6 should fail.
[5] Apply the patch.
[6] Run the test case again; this time, all tests should pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 6 Galen Charlton 2013-06-27 15:28:02 UTC
Pushed to master.
Comment 7 Jonathan Druart 2013-06-27 15:32:40 UTC
(In reply to Galen Charlton from comment #6)
> Pushed to master.

Thanks, Galen! :)
Comment 8 Tomás Cohen Arazi 2013-07-15 18:07:52 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks Galen!
Comment 9 Bernardo Gonzalez Kriegel 2013-07-16 11:58:49 UTC
Pushed to 3.10.x, will be in 3.10.8