[Bug 14256] New: tests for TestBuider randomly fails
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Bug ID: 14256 Summary: tests for TestBuider randomly fails Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Test Suite Assignee: jonathan.druart@biblibre.com Reporter: jonathan.druart@biblibre.com QA Contact: gmcharlt@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14195 Depends on| |12603 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- To reproduce diff --git a/t/db_dependent/TestBuilder.t b/t/db_dependent/TestBuilder.t index bec4834..93e3fb8 100644 --- a/t/db_dependent/TestBuilder.t +++ b/t/db_dependent/TestBuilder.t @@ -31,8 +31,9 @@ is( $builder->build(), undef, 'build without arguments returns undef' ); my @sources = $builder->schema->sources; my $nb_failure = 0; +@sources = ('Koha::Schema::Result::Borrower') x 1000; for my $source (@sources) { - eval { $builder->build( { source => $source } ); }; + $builder->build( { source => $source } ); $nb_failure++ if ($@); } is( $nb_failure, 0, 'TestBuilder can create a entry for every sources' ); -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #2 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 39430 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39430&action=edit Bug 14256: Make TestBuilder regenerates a string if to short In some cases, especially branchcode and categorycode, the generated string for a primary key already existed in DB. Which make the tests fail, and the data are not gererated. This patch regenerate a string if the one generated is too short and can be longer. Test plan: Use the diff from the previous comment, and confirm that the tests only fail in the very few cases. QA: Actually I don't know if the better could be to use the existing value, but maybe not... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #2)
QA: Actually I don't know if the better could be to use the existing value, but maybe not...
We shouldn't reuse the data. To me, the point of TestBuilder is to avoid side-effects while testing: i.e. avoid situations in which we think our code works, but is actually working only because of side-effects. TestBuilder provides a simple approach to writing integration tests. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch Summary|tests for TestBuider |tests for TestBuilder |randomly fails |randomly fails -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtompset@hotmail.com --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- What did you have in the categories table by default? I'm getting explosions "all" the time. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to M. Tompsett from comment #4)
What did you have in the categories table by default? I'm getting explosions "all" the time.
"All the time": with or without the changes on comment 1? I always use the sample data to launch tests. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- mtompset@debian:~/kohaclone$ prove t/db_dependent/TestBuilder.t t/db_dependent/TestBuilder.t .. 1/41 DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_library`.`borrower_attribute_types`, CONSTRAINT `category_code_fk` FOREIGN KEY (`category_code`) REFERENCES `categories` (`categorycode`)) at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_library`.`borrower_attribute_types`, CONSTRAINT `category_code_fk` FOREIGN KEY (`category_code`) REFERENCES `categories` (`categorycode`)) at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_library`.`borrower_attribute_types`, CONSTRAINT `category_code_fk` FOREIGN KEY (`category_code`) REFERENCES `categories` (`categorycode`)) at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. t/db_dependent/TestBuilder.t .. 3/41 With and without the patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #7 from M. Tompsett <mtompset@hotmail.com> --- (In reply to M. Tompsett from comment #6)
With and without the patch.
bug fix patch... now to try with comment #1 tweak, but I'm sure this is unrelated. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to M. Tompsett from comment #7)
(In reply to M. Tompsett from comment #6)
With and without the patch.
bug fix patch... now to try with comment #1 tweak, but I'm sure this is unrelated.
Please try not to make the comment list too long as it will become useless. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39430|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 39463 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39463&action=edit Bug 14256: Make TestBuilder regenerates a string if to short In some cases, especially branchcode and categorycode, the generated string for a primary key already existed in DB. Which make the tests fail, and the data are not gererated. This patch regenerate a string if the one generated is too short and can be longer. Test plan: Use the diff from the previous comment, and confirm that the tests only fail in the very few cases. QA: Actually I don't know if the better could be to use the existing value, but maybe not... Signed-off-by: Mark Tompsett <mtompset@hotmail.com> NOTE: Perhaps a hardcoded size like >=2 could be replaced by the minimum of (+1 max string length in the field, size cap). After a couple glitches testing, my bad, it does reduce failures drastically. I had category code of length 4 in my database at the time. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39463|0 |1 is obsolete| | --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 39788 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39788&action=edit Bug 14256: Make TestBuilder regenerates a string if to short In some cases, especially branchcode and categorycode, the generated string for a primary key already existed in DB. Which make the tests fail, and the data are not gererated. This patch regenerate a string if the one generated is too short and can be longer. Test plan: Use the diff from the previous comment, and confirm that the tests only fail in the very few cases. QA: Actually I don't know if the better could be to use the existing value, but maybe not... Signed-off-by: Mark Tompsett <mtompset@hotmail.com> NOTE: Perhaps a hardcoded size like >=2 could be replaced by the minimum of (+1 max string length in the field, size cap). After a couple glitches testing, my bad, it does reduce failures drastically. I had category code of length 4 in my database at the time. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 39789 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39789&action=edit Bug 14256: (followup) do not repeat existing values on primary columns This followup patch makes _buildColumnValue test the generated random value so TestBuilder doesn't try to insert repeated values on primary columns. It also removes an unnecesary use of 'eval' that made it difficult to spot errors. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|tests for TestBuilder |Tests for TestBuilder fail |randomly fails |randomly -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- Comment on attachment 39789 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39789 Bug 14256: (followup) do not repeat existing values on primary columns Review of attachment 39789: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14256&attachment=39789) ----------------------------------------------------------------- ::: t/lib/TestBuilder.pm @@ +244,5 @@
+ my $data_type = $col_info->{data_type}; + $data_type =~ s| |_|; + + if ( any { $data_type eq $_ } (keys $gen_type) ) { + my $value_ok = 'no';
A boolean would be enough. @@ +249,5 @@
+ while ( $value_ok eq 'no' ) { + # generate value + $col_value = $gen_type->{$data_type}->( $self, { info => $col_info } ); + # should value be unique? + my $primary_names = $self->schema->source($source)->primary_columns();
What about unique keys? @@ +250,5 @@
+ # generate value + $col_value = $gen_type->{$data_type}->( $self, { info => $col_info } ); + # should value be unique? + my $primary_names = $self->schema->source($source)->primary_columns(); + if ( not ( any { $col_name eq $_ } $primary_names and
any takes a list in parameter, not a ref. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39789|0 |1 is obsolete| | --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 39824 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39824&action=edit Bug 14256: (followup) check for unique constrainta to regenerate random data Unique constraints should be checked when creating random data. Otherwise we get failures when the generated data already exists on the DB. This patch takes advantage of ->unique_constraints() to do the job, looping through all the unique constraints defined for the source. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|gmcharlt@gmail.com |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39824|0 |1 is obsolete| | --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 39825 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39825&action=edit Bug 14256: (followup) check for unique constrainta to regenerate random data Unique constraints should be checked when creating random data. Otherwise we get failures when the generated data already exists on the DB. This patch takes advantage of ->unique_constraints() to do the job, looping through all the unique constraints defined for the source. Edit: removed leftover variable -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #15 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 39825 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39825 Bug 14256: (followup) check for unique constrainta to regenerate random data Review of attachment 39825: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14256&attachment=39825) ----------------------------------------------------------------- ::: t/lib/TestBuilder.pm @@ +192,5 @@
+ } + + my $count = $self->schema + ->resultset( $source ) + ->search( $condition )
Making the changes suggested in comment #1 sometimes triggers a KABOOM here, because condition is not right somehow. I think something goes wrong with the building loop above. { ARRAY => undef }?! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39788|0 |1 is obsolete| | Attachment #39825|0 |1 is obsolete| | --- Comment #16 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 39835 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39835&action=edit Bug 14256: Check for unique constraint to regenerate random data Unique constraints should be checked when creating random data. Otherwise we get failures when the generated data already exists on the DB. This patch takes advantage of ->unique_constraints() to do the job, looping through all the unique constraints defined for the source. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #17 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 39836 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39836&action=edit Bug 14256: (follow-up) Check for unique constraint to regenerate random data There were some issues in the previous patch. This patch fixes the following: - rename $value with $original_value - remove $at_least_one_constraint_failed and $values_ok which make the code unnecessarily complicated - the constraints have to be checked only if no original value is passed - _buildColumnValue created a key to the default value hashref, it broke the test: last BUILD_VALUE if exists( $default_value->{$source} ); -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39835|0 |1 is obsolete| | --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 39854 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39854&action=edit [SIGNED OFF] Bug 14256: Check for unique constraint to regenerate random data Unique constraints should be checked when creating random data. Otherwise we get failures when the generated data already exists on the DB. This patch takes advantage of ->unique_constraints() to do the job, looping through all the unique constraints defined for the source. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39836|0 |1 is obsolete| | --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 39855 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39855&action=edit [SIGNED OFF] Bug 14256: (follow-up) Check for unique constraint to regenerate random data There were some issues in the previous patch. This patch fixes the following: - rename $value with $original_value - remove $at_least_one_constraint_failed and $values_ok which make the code unnecessarily complicated - the constraints have to be checked only if no original value is passed - _buildColumnValue created a key to the default value hashref, it broke the test: last BUILD_VALUE if exists( $default_value->{$source} ); Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Marking as signed-off. The first patch was a followup for Jonathan's... Im not sure about the workflow here, but it would be enough if some dbix experienced dev like Kyle does QA on it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39854|0 |1 is obsolete| | Attachment #39855|0 |1 is obsolete| | --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 40041 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40041&action=edit [PASSED QA] Bug 14256: Check for unique constraint to regenerate random data Unique constraints should be checked when creating random data. Otherwise we get failures when the generated data already exists on the DB. This patch takes advantage of ->unique_constraints() to do the job, looping through all the unique constraints defined for the source. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 40042 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40042&action=edit [PASSED QA] Bug 14256: (follow-up) Check for unique constraint to regenerate random data There were some issues in the previous patch. This patch fixes the following: - rename $value with $original_value - remove $at_least_one_constraint_failed and $values_ok which make the code unnecessarily complicated - the constraints have to be checked only if no original value is passed - _buildColumnValue created a key to the default value hashref, it broke the test: last BUILD_VALUE if exists( $default_value->{$source} ); Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 --- Comment #23 from Kyle M Hall <kyle@bywatersolutions.com> --- Looks good! It definitely fixes the issue and I see no particular problems with it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14256 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |chris@bigballofwax.co.nz --- Comment #25 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.20.x will be in 3.20.1 (note for other RMaints, not needed on other branches) -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org