Bug 19386 - t/db_dependent/SIP/Patron.t is failing randomly
Summary: t/db_dependent/SIP/Patron.t is failing randomly
Status: Pushed to Stable
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: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-28 20:30 UTC by Jonathan Druart
Modified: 2017-11-23 23:23 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19386: Prevent t/db_dependent/SIP/Patron.t to fail randomly (2.65 KB, patch)
2017-09-28 20:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19386: Prevent t/db_dependent/SIP/Patron.t to fail randomly (2.71 KB, patch)
2017-09-29 17:58 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 19386: Prevent t/db_dependent/SIP/Patron.t to fail randomly (2.80 KB, patch)
2017-10-06 09:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19386: [QA Follow-up] Remove some default values (1.33 KB, patch)
2017-10-06 09:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-09-28 20:30:53 UTC
Depending on the values used by TestBuilder, some tests from t/db_dependent/SIP/Patron.t can fail

    #   Failed test 'Not blocked without overdues when set to 'Don't block''
    #   at t/db_dependent/SIP/Patron.t line 55.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Not blocked without overdues when set to 'confirmation''
    #   at t/db_dependent/SIP/Patron.t line 61.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Not blocked without overdues when set to 'Block''
    #   at t/db_dependent/SIP/Patron.t line 67.
    #          got: '0'
    #     expected: '1'
Comment 1 Jonathan Druart 2017-09-28 20:33:32 UTC
Created attachment 67432 [details] [review]
Bug 19386: Prevent t/db_dependent/SIP/Patron.t to fail randomly

Depending on the values used by TestBuilder, some tests from t/db_dependent/SIP/Patron.t can fail

    #   Failed test 'Not blocked without overdues when set to 'Don't block''
    #   at t/db_dependent/SIP/Patron.t line 55.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Not blocked without overdues when set to 'confirmation''
    #   at t/db_dependent/SIP/Patron.t line 61.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Not blocked without overdues when set to 'Block''
    #   at t/db_dependent/SIP/Patron.t line 67.
    #          got: '0'
    #     expected: '1'

Caused by this check in C4::SIP::ILS::Patron->new
110     for (qw(EXPIRED CHARGES CREDITS GNA LOST DBARRED NOTES)) {
111         ($flags->{$_}) or next;
[...]
115         if ($flags->{$_}->{noissues}) {
116             foreach my $toggle (qw(charge_ok renew_ok recall_ok hold_ok inet)) {
117                 $ilspatron{$toggle} = 0;    # if we get noissues, disable everything
118             }
119         }
120     }

Test plan:
Execute several times (50x?) these tests.
Without this patch they may fail
With this patch they must never fail
Comment 2 Jonathan Druart 2017-09-28 20:34:03 UTC
Note: We should certainly add these values as default for TestBuilder
Comment 3 David Bourgault 2017-09-29 17:58:36 UTC
Created attachment 67481 [details] [review]
Bug 19386: Prevent t/db_dependent/SIP/Patron.t to fail randomly

Depending on the values used by TestBuilder, some tests from t/db_dependent/SIP/Patron.t can fail

    #   Failed test 'Not blocked without overdues when set to 'Don't block''
    #   at t/db_dependent/SIP/Patron.t line 55.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Not blocked without overdues when set to 'confirmation''
    #   at t/db_dependent/SIP/Patron.t line 61.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Not blocked without overdues when set to 'Block''
    #   at t/db_dependent/SIP/Patron.t line 67.
    #          got: '0'
    #     expected: '1'

Caused by this check in C4::SIP::ILS::Patron->new
110     for (qw(EXPIRED CHARGES CREDITS GNA LOST DBARRED NOTES)) {
111         ($flags->{$_}) or next;
[...]
115         if ($flags->{$_}->{noissues}) {
116             foreach my $toggle (qw(charge_ok renew_ok recall_ok hold_ok inet)) {
117                 $ilspatron{$toggle} = 0;    # if we get noissues, disable everything
118             }
119         }
120     }

Test plan:
Execute several times (50x?) these tests.
Without this patch they may fail
With this patch they must never fail

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Comment 4 David Bourgault 2017-09-29 17:59:42 UTC
Test 100 hundred times before and after.
Before it failed 6 times, after it did not.

Command used:
for i in {1..100}; do prove t/db_dependent/SIP/Patron.t | grep Failed; done
Comment 5 Marcel de Rooy 2017-10-06 09:42:29 UTC
Created attachment 67682 [details] [review]
Bug 19386: Prevent t/db_dependent/SIP/Patron.t to fail randomly

Depending on the values used by TestBuilder, some tests from t/db_dependent/SIP/Patron.t can fail

    #   Failed test 'Not blocked without overdues when set to 'Don't block''
    #   at t/db_dependent/SIP/Patron.t line 55.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Not blocked without overdues when set to 'confirmation''
    #   at t/db_dependent/SIP/Patron.t line 61.
    #          got: '0'
    #     expected: '1'

    #   Failed test 'Not blocked without overdues when set to 'Block''
    #   at t/db_dependent/SIP/Patron.t line 67.
    #          got: '0'
    #     expected: '1'

Caused by this check in C4::SIP::ILS::Patron->new
110     for (qw(EXPIRED CHARGES CREDITS GNA LOST DBARRED NOTES)) {
111         ($flags->{$_}) or next;
[...]
115         if ($flags->{$_}->{noissues}) {
116             foreach my $toggle (qw(charge_ok renew_ok recall_ok hold_ok inet)) {
117                 $ilspatron{$toggle} = 0;    # if we get noissues, disable everything
118             }
119         }
120     }

Test plan:
Execute several times (50x?) these tests.
Without this patch they may fail
With this patch they must never fail

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2017-10-06 09:42:33 UTC
Created attachment 67683 [details] [review]
Bug 19386: [QA Follow-up] Remove some default values

In the meantime bug 19403 added default nulls for gonenoaddress, lost
and debarred. So these lines can be removed safely.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2017-10-06 15:19:30 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 8 Fridolin SOMERS 2017-10-27 13:21:15 UTC
Pushed to 17.05.x, will be in 17.05.06.
Comment 9 Katrin Fischer 2017-10-29 18:48:56 UTC
These patches have been pushed to 16.11.x and will be in 16.11.14.
Comment 10 Mason James 2017-11-23 23:23:42 UTC
Pushed to 16.05.x, for 16.05.19 release