Bug 19463 - TestBuilder.t is failing randomly
Summary: TestBuilder.t is failing randomly
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-13 15:59 UTC by Jonathan Druart
Modified: 2017-10-18 12:08 UTC (History)
1 user (show)

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


Attachments
Bug 19463: Reduce the chance to have TestBuilder generate twice the same value (1.72 KB, patch)
2017-10-17 14:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19463: Reduce the chance to have TestBuilder generate twice the same value (1.60 KB, patch)
2017-10-17 15:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19463: Additional changes to gen_text (1.86 KB, patch)
2017-10-17 15:02 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19463: (Follow-up) Cosmetic changes (1.34 KB, patch)
2017-10-18 12:08 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-10-13 15:59:54 UTC
Violation of unique constraint in Branch at /kohadevbox/koha/t/lib/TestBuilder.pm line 233.

        #   Failed test 'Testing Koha::Old::Checkouts'
        #   at t/db_dependent/TestBuilder.t line 389.
        #          got: ''
        #     expected: 'Koha::Old::Checkout'
        # Looks like you failed 1 test of 67.

    #   Failed test 'Test all classes'
    #   at t/db_dependent/TestBuilder.t line 391.
    # Looks like you failed 1 test of 6.

#   Failed test 'build_object() tests'
#   at t/db_dependent/TestBuilder.t line 392.
# Looks like you failed 1 test of 12.
[07:55:06] t/db_dependent/TestBuilder.t ................................... 

I guess it tries to generate twice the same value branchcode (unlucky!)
Comment 1 Marcel de Rooy 2017-10-16 07:11:00 UTC
(In reply to Jonathan Druart from comment #0)
> Violation of unique constraint in Branch at
> /kohadevbox/koha/t/lib/TestBuilder.pm line 233.
> 
> I guess it tries to generate twice the same value branchcode (unlucky!)

It tries three times now. Random is not always that random.. Normally, the two additional tries are sufficient, but I have seen it happen too.
We could increase that number?
Comment 2 Jonathan Druart 2017-10-16 11:51:50 UTC
That would mean we have too many objects created then...
Comment 3 Jonathan Druart 2017-10-16 15:31:44 UTC
Violation of unique constraint in Category at /kohadevbox/koha/t/lib/TestBuilder.pm line 240.

        #   Failed test 'Testing Koha::Ratings'
        #   at t/db_dependent/TestBuilder.t line 390.
        #          got: ''
        #     expected: 'Koha::Rating'
        # Looks like you failed 1 test of 70.

    #   Failed test 'Test all classes'
    #   at t/db_dependent/TestBuilder.t line 392.
    # Looks like you failed 1 test of 6.

#   Failed test 'build_object() tests'
#   at t/db_dependent/TestBuilder.t line 393.
# Looks like you failed 1 test of 13.
[15:07:54] t/db_dependent/TestBuilder.t
Comment 4 Jonathan Druart 2017-10-17 14:31:57 UTC
Created attachment 68217 [details] [review]
Bug 19463: Reduce the chance to have TestBuilder generate twice the same value

When an id is generated by TestBuilder (branchcode for instance) and the
size of the generated string is 1, we have too many chances to get
"Violation of unique constraint in Branch".

This patch will enforce the size of the string to be > 1 and the number
of tries to get a different strings will be 5 (instead of 3).
Comment 5 Marcel de Rooy 2017-10-17 15:02:48 UTC
Created attachment 68218 [details] [review]
Bug 19463: Reduce the chance to have TestBuilder generate twice the same value

When an id is generated by TestBuilder (branchcode for instance) and the
size of the generated string is 1, we have too many chances to get
"Violation of unique constraint in Branch".

This patch increases the number of retries to 5.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended to make room for follow-up.
Comment 6 Marcel de Rooy 2017-10-17 15:02:52 UTC
Created attachment 68219 [details] [review]
Bug 19463: Additional changes to gen_text

Use Bytes::Random::Secure instead of perl rand.
Return a string from 50 to 100% of $size.
Comment 7 Jonathan Druart 2017-10-17 15:17:54 UTC
Thanks Marcel!

Pushed to master for 17.11.
Comment 8 Marcel de Rooy 2017-10-18 12:08:15 UTC
Created attachment 68244 [details] [review]
Bug 19463: (Follow-up) Cosmetic changes

String::Random version 0.26 (on Jessie) does not yet support the rand_gen
parameter (0.27 does, newest is 0.29 on CPAN now).
So alt_rand is only used in determining the size on Jessie.
That might be enough though.
Adding a documention line in this regard.

Removing the obsolete max parameter.

Note: I timed alt_rand for the creation of a new Bytes::Random::Secure
object each time. But each call is about 0.1 milliseconds. So that
should be fine.

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