Bug 14195 - TestBuilder does not generate the good length for text fields
Summary: TestBuilder does not generate the good length for text fields
Status: CLOSED FIXED
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: 12603
Blocks: 14045
  Show dependency treegraph
 
Reported: 2015-05-13 13:41 UTC by Jonathan Druart
Modified: 2016-12-05 21:23 UTC (History)
3 users (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 14105: TestBuilder - A random string should not be longer than the DB field (2.17 KB, patch)
2015-05-13 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
To reproduce the issue (471 bytes, application/x-perl)
2015-05-13 14:44 UTC, Jonathan Druart
Details
[SIGNED-OFF] Bug 14195: TestBuilder - A random string should not be longer than the DB field (2.33 KB, patch)
2015-05-14 18:40 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 14195: (followup) increment number of tests (775 bytes, patch)
2015-05-14 18:40 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 14195: TestBuilder - A random string should not be longer than the DB field (2.42 KB, patch)
2015-05-20 11:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 14195: (followup) increment number of tests (874 bytes, patch)
2015-05-20 11:18 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 2015-05-13 13:41:32 UTC
t::lib::TestBuilder::_gen_text does not use correctly the regex and the max parameter to generate the random string (String::Random).
Comment 1 Jonathan Druart 2015-05-13 14:42:30 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-05-13 14:44:35 UTC
Created attachment 39139 [details]
To reproduce the issue

Launch this script several times, without applying the patch.
If the generated string for categorycode has a length > 10, you will see a difference.

After applying the patch, the difference should never occur.
Comment 3 Bernardo Gonzalez Kriegel 2015-05-14 18:40:30 UTC Comment hidden (obsolete)
Comment 4 Bernardo Gonzalez Kriegel 2015-05-14 18:40:51 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2015-05-19 12:59:06 UTC
Small question before passing QA: why do you decrement $size here:

+    if ( defined $size and $size > 1 ) {
+        $size--;
Comment 6 Jonathan Druart 2015-05-19 14:36:09 UTC
(In reply to Marcel de Rooy from comment #5)
> Small question before passing QA: why do you decrement $size here:
> 
> +    if ( defined $size and $size > 1 ) {
> +        $size--;

Because the regex, in this case, is
    [A-Za-z][A-Za-z0-9_]*
And the max size passed to String::Random is related to the wild card, not the the size of the complete string.
Comment 7 Marcel de Rooy 2015-05-20 11:18:38 UTC
Created attachment 39340 [details] [review]
Bug 14195: TestBuilder - A random string should not be longer than the DB field

t::lib::TestBuilder::_gen_text does not use correctly the regex and the
max parameter to generate the random string (String::Random).

This can cause future tests to fail.

http://bugs.koha-community.org/show_bug.cgi?id=14195
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Script tested, problem occurs, patch fixes it.
Bad number on commit subject
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2015-05-20 11:18:43 UTC
Created attachment 39341 [details] [review]
Bug 14195: (followup) increment number of tests

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Marcel de Rooy 2015-05-20 11:20:18 UTC
(In reply to Jonathan Druart from comment #6)

> Because the regex, in this case, is
>     [A-Za-z][A-Za-z0-9_]*
> And the max size passed to String::Random is related to the wild card, not
> the the size of the complete string.

Fine. Could have read this in your comment :) Just not very intuitive Random parameter.. Thanks.
Comment 10 Tomás Cohen Arazi 2015-05-20 14:23:30 UTC
Patch pushed to master.

Thanks Jonathan!