Bug 17759 - Fixing theoretical problems with guarantorid in Members.t
Summary: Fixing theoretical problems with guarantorid in Members.t
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Galen Charlton
URL:
Keywords:
Depends on: 17733
Blocks:
  Show dependency treegraph
 
Reported: 2016-12-12 11:50 UTC by Marcel de Rooy
Modified: 2018-06-04 20:09 UTC (History)
3 users (show)

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


Attachments
Bug 17759: Remove invalid guarantorid's in Members.t (3.13 KB, patch)
2016-12-12 13:11 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17759: Remove invalid guarantorid's in Members.t (3.15 KB, patch)
2016-12-12 13:39 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2016-12-12 11:50:49 UTC
This comes from bug 17733:

I manipulated guarantorid of some borrower (with your calculation ;)
And the first time I ran the test, it failed:

not ok 65 - TrackLastPatronActivity - 2 patrons must be deleted
#   Failed test 'TrackLastPatronActivity - 2 patrons must be deleted'
#   at t/db_dependent/Members.t line 363.
#          got: '1'
#     expected: '2'
not ok 66 - TrackLastPatronActivity - 3 patrons must be deleted
#   Failed test 'TrackLastPatronActivity - 3 patrons must be deleted'
#   at t/db_dependent/Members.t line 365.
#          got: '2'
#     expected: '3'

As expected, the test did not fail when running it again.
So you need to remove some guarantorids here too. GetBorrowersToExpunge checks for this column.
Comment 1 Marcel de Rooy 2016-12-12 13:11:03 UTC
Created attachment 58117 [details] [review]
Bug 17759: Remove invalid guarantorid's in Members.t

See also bug 17733. As long as we have no FK, these invalid guarantorid's
can make tests fail.
Adding a sql statement to clear them in the beginning of the test.

Test plan:
[1] Since it seems that AUTO_INC+8, +9 and +10 may fail one or two tests,
    we should manipulate one or two borrowers:
    Run this SQL query first:
SELECT `AUTO_INCREMENT` FROM  INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = '[your database]' and table_name='borrowers';
    Now run: UPDATE borrowers SET guarantorid=[former value + 8] WHERE borrowernumber=[some existing borrowernumber]
[2] Without this patch, run Members.t. Should fail two tests.
[3] Apply the patch. Fetch the new AUTO_INCREMENT value and update
    guarantorid accordingly again for the manipulated borrower.
[4] Run the test. It should not fail.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Jonathan Druart 2016-12-12 13:39:45 UTC
Created attachment 58119 [details] [review]
Bug 17759: Remove invalid guarantorid's in Members.t

See also bug 17733. As long as we have no FK, these invalid guarantorid's
can make tests fail.
Adding a sql statement to clear them in the beginning of the test.

Test plan:
[1] Since it seems that AUTO_INC+8, +9 and +10 may fail one or two tests,
    we should manipulate one or two borrowers:
    Run this SQL query first:
SELECT `AUTO_INCREMENT` FROM  INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = '[your database]' and table_name='borrowers';
    Now run: UPDATE borrowers SET guarantorid=[former value + 8] WHERE borrowernumber=[some existing borrowernumber]
[2] Without this patch, run Members.t. Should fail two tests.
[3] Apply the patch. Fetch the new AUTO_INCREMENT value and update
    guarantorid accordingly again for the manipulated borrower.
[4] Run the test. It should not fail.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 3 Jonathan Druart 2016-12-12 13:40:20 UTC
Skipping QA, too much time spent on these tests already.
Comment 4 Kyle M Hall 2016-12-16 11:26:12 UTC
Pushed to master for 17.05, thanks Marcel!
Comment 5 Katrin Fischer 2016-12-18 20:31:30 UTC
This patch has been pushed to 16.11.x, will be in 16.11.01.