One unit test for Koha::Patrons->update_category_to seems wrong : is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode_2,too_young=>1})->update_category_to({category=>$a_categorycode}),1,'One child patron updated to adult category'); Child is updated to adult because he his too old, not too young.
Created attachment 115849 [details] [review] Bug 27554: Add a test for Koha::Patrons->update_category_to child to adult One unit test for Koha::Patrons->update_category_to seems wrong : is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode_2,too_young=>1})->update_category_to({category=>$a_categorycode}),1,'One child patron updated to adult category'); Child is updated to adult because he his too old, not too young. This patch adds a test on category of patron that should be an adult after update. Test plan : 1) Without patch, run t/db_dependent/Patrons.t 2) It does not fail 3) Apply patch, run t/db_dependent/Patrons.t 4) It fails on test 'The over age patron has been updated to adult category'
Created attachment 115850 [details] [review] Bug 27554: Fix test for Koha::Patrons->update_category_to child to adult This patch fixes the test by using too_old instead of too_young Test plan : Apply patch, run t/db_dependent/Patrons.t, tests are OK
Created attachment 115860 [details] [review] Bug 27554: Add a test for Koha::Patrons->update_category_to child to adult One unit test for Koha::Patrons->update_category_to seems wrong : is( Koha::Patrons->search_patrons_to_update_category({from=>$c_categorycode_2,too_young=>1})->update_category_to({category=>$a_categorycode}),1,'One child patron updated to adult category'); Child is updated to adult because he his too old, not too young. This patch adds a test on category of patron that should be an adult after update. Test plan : 1) Without patch, run t/db_dependent/Patrons.t 2) It does not fail 3) Apply patch, run t/db_dependent/Patrons.t 4) It fails on test 'The over age patron has been updated to adult category' Signed-off-by: David Nind <david@davidnind.com>
Created attachment 115861 [details] [review] Bug 27554: Fix test for Koha::Patrons->update_category_to child to adult This patch fixes the test by using too_old instead of too_young Test plan : Apply patch, run t/db_dependent/Patrons.t, tests are OK Signed-off-by: David Nind <david@davidnind.com>
I see what you thought here, but actually the patron was updated because they were too young child1 is 4 years old - too young for child account The script will move you to the next categry without checking the age limits for that category - i believe this is how the past script worked too. It is not a logical test, but valid child3 is 18 - you make your tests affect this patron instead - maybe we want your test in addition to the existing tests?
(In reply to Nick Clemens from comment #5) > I see what you thought here, but actually the patron was updated because > they were too young > > child1 is 4 years old - too young for child account > > The script will move you to the next category without checking the age limits > for that category - i believe this is how the past script worked too. It is > not a logical test, but valid OK but I'm working on adding a check on age limits so it breaks this test. > > child3 is 18 - you make your tests affect this patron instead - maybe we > want your test in addition to the existing tests? Yes, in my opinion test should verify which patron is impacted. But the test also checks that a child looses its guarantor so it must be from child to adult. Why not fix the test ? It would me more clear in case it fails.
Created attachment 115933 [details] [review] Bug 27554: Clarify and add tests This patch adds a comment to the existing tests, clarifying what they test, and verifying the expected patron is affected. The original test should be broken by 26307 when the behvaiour is prevented I additionally add a test for 'too old' To test: 1 - Apply patch 2 - Prove tests, they should pass 3 - Verify the comments make sense
Created attachment 115934 [details] [review] Bug 27554: Clarify and add tests This patch adds a comment to the existing tests, clarifying what they test, and verifying the expected patron is affected. The original test should be broken by 26307 when the behvaiour is prevented I additionally add a test for 'too old' To test: 1 - Apply patch 2 - Prove tests, they should pass 3 - Verify the comments make sense Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 116097 [details] [review] Bug 27554: Clarify and add tests This patch adds a comment to the existing tests, clarifying what they test, and verifying the expected patron is affected. The original test should be broken by 26307 when the behvaiour is prevented I additionally add a test for 'too old' To test: 1 - Apply patch 2 - Prove tests, they should pass 3 - Verify the comments make sense Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Nice cooperation here :)
Created attachment 116171 [details] [review] Bug 27554: Clarify and add tests for update_category_to This patch adds a comment to the existing tests, clarifying what they test, and verifying the expected patron is affected. The original test should be broken by 26307 when the behvaiour is prevented I additionally add a test for 'too old' To test: 1 - Apply patch 2 - Prove tests, they should pass 3 - Verify the comments make sense Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> JD amended patch: behvaiour ==> behaviour and add more info to the commit title
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.03
Pushed to 20.05.x for 20.05.09
Backported: Pushed to 19.11.x branch for 19.11.15