Tests in bug 22284 had conditions where they randomly fail
Created attachment 98883 [details] [review] Bug 24657: Fix t/db_dependent/Koha/Item.t and t/db_dependent/Holds.t on t/db_dependent/Koha/Item.t on line 172 I created 2 Koha::Library::Groups like this my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); I didn't realize this was creating 2 new libraries that sometimes messed up with tests, so I changed it to this my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } ); my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } ); on t/db_dependent/Holds.t on line 1058 I created 3 libraries like this my $library1 = $builder->build_object( { class => 'Koha::Libraries' } ); my $library2 = $builder->build_object( { class => 'Koha::Libraries' } ); my $library3 = $builder->build_object( { class => 'Koha::Libraries' } ); but they needed to be pickup_locations, and sometimes they wheren't set as such, so I changed it to this my $library1 = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } ); my $library2 = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } ); my $library3 = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } ); To test: 1. do not apply this patch 2. in bash: for i in {1..300}; do echo "loop $i"; prove t/db_dependent/Koha/Item.t t/db_dependent/Holds.t; if [ "$?" = "1" ]; then break; fi; done 3. Grab a cup of coffee (or tea if you are healthy) and wait for a while 4. Whithin 300 iterations there should be an error in any of both scripts and for loop should exit 5. Apply this patch 6. repeat step 2 and 3 (decaff this time!) 7. All 300 loops should pass 8. Sign off
Created attachment 98884 [details] [review] Bug 24657: Fix t/db_dependent/Koha/Item.t and t/db_dependent/Holds.t on t/db_dependent/Koha/Item.t on line 172 I created 2 Koha::Library::Groups like this my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); I didn't realize this was creating 2 new libraries that sometimes messed up with tests, so I changed it to this my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } ); my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } ); on t/db_dependent/Holds.t on line 1058 I created 3 libraries like this my $library1 = $builder->build_object( { class => 'Koha::Libraries' } ); my $library2 = $builder->build_object( { class => 'Koha::Libraries' } ); my $library3 = $builder->build_object( { class => 'Koha::Libraries' } ); but they needed to be pickup_locations, and sometimes they wheren't set as such, so I changed it to this my $library1 = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } ); my $library2 = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } ); my $library3 = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } ); To test: 1. do not apply this patch 2. in bash: for i in {1..300}; do echo "loop $i"; prove t/db_dependent/Koha/Item.t t/db_dependent/Holds.t; if [ "$?" = "1" ]; then break; fi; done 3. Grab a cup of coffee (or tea if you are healthy) and wait for a while 4. Whithin 300 iterations there should be an error in any of both scripts and for loop should exit 5. Apply this patch 6. repeat step 2 and 3 (decaff this time!) 7. All 300 loops should pass 8. Sign off
Comment on attachment 98884 [details] [review] Bug 24657: Fix t/db_dependent/Koha/Item.t and t/db_dependent/Holds.t Review of attachment 98884 [details] [review]: ----------------------------------------------------------------- ::: t/db_dependent/Koha/Item.t @@ +173,5 @@ > + my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } ); > + my $library1 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1, branchcode => 'TEST1' } } ); > + my $library2 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1, branchcode => 'TEST2' } } ); > + my $library3 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 0, branchcode => 'TEST3' } } ); > + my $library4 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1, branchcode => 'TEST4' } } ); Do you really need to specify a branchcode here?
I got another random failure, but not related to bug 22284. First time I see it I think. For the record: t/db_dependent/Holds.t ...... 1/61 # Failed test 'Test ModReserveMinusPriority()' # at t/db_dependent/Holds.t line 203. # got: undef # expected: '2304' t/db_dependent/Holds.t ...... 61/61 # Looks like you failed 1 test of 61.
Created attachment 98917 [details] [review] Bug 24657: Fix t/db_dependent/Koha/Item.t and t/db_dependent/Holds.t on t/db_dependent/Koha/Item.t on line 172 I created 2 Koha::Library::Groups like this my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1 } } ); I didn't realize this was creating 2 new libraries that sometimes messed up with tests, so I changed it to this my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } ); my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } ); on t/db_dependent/Holds.t on line 1058 I created 3 libraries like this my $library1 = $builder->build_object( { class => 'Koha::Libraries' } ); my $library2 = $builder->build_object( { class => 'Koha::Libraries' } ); my $library3 = $builder->build_object( { class => 'Koha::Libraries' } ); but they needed to be pickup_locations, and sometimes they wheren't set as such, so I changed it to this my $library1 = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } ); my $library2 = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } ); my $library3 = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } ); To test: 1. do not apply this patch 2. in bash: for i in {1..300}; do echo "loop $i"; prove t/db_dependent/Koha/Item.t t/db_dependent/Holds.t; if [ "$?" = "1" ]; then break; fi; done 3. Grab a cup of coffee (or tea if you are healthy) and wait for a while 4. Whithin 300 iterations there should be an error in any of both scripts and for loop should exit 5. Apply this patch 6. repeat step 2 and 3 (decaff this time!) 7. All 300 loops should pass 8. Sign off Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 98918 [details] [review] Bug 24657: Do not explicitely pass a branchcode Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Thanks Agustin, good catch!
(In reply to Jonathan Druart from comment #3) > Comment on attachment 98884 [details] [review] [review] > Bug 24657: Fix t/db_dependent/Koha/Item.t and t/db_dependent/Holds.t > > Review of attachment 98884 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: t/db_dependent/Koha/Item.t > @@ +173,5 @@ > > + my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_hold_group => 1, branchcode => undef } } ); > > + my $library1 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1, branchcode => 'TEST1' } } ); > > + my $library2 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1, branchcode => 'TEST2' } } ); > > + my $library3 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 0, branchcode => 'TEST3' } } ); > > + my $library4 = $builder->build_object( { class => 'Koha::Libraries', value => { pickup_location => 1, branchcode => 'TEST4' } } ); > > Do you really need to specify a branchcode here? Ups, that slipped away.. I used it to verify which libraries where tested, and when a library without one of those branchcodes appeared I realized that somewhere they where created. Thanks for the patch
Tests only and all look like sane and sensible changes to me. Passing QA
Nice work everyone! Pushed to master for 20.05
enhancement not backported to 19.11.x
(In reply to Jonathan Druart from comment #4) > I got another random failure, but not related to bug 22284. First time I see > it I think. > For the record: > t/db_dependent/Holds.t ...... 1/61 > # Failed test 'Test ModReserveMinusPriority()' > # at t/db_dependent/Holds.t line 203. > # got: undef > # expected: '2304' > t/db_dependent/Holds.t ...... 61/61 # Looks like you failed 1 test of 61. See bug 26043