Created attachment 58005 [details] [review] Bug 17733: Fix Members.t In some dirty DB (Jenkins), the borrowers.guarantorid field is not set to a valid patron id. That make some tests fail if we create a patron with that patron id. In my DB, auto increment for borrowers is set to 52 before the tests On the 4th run of the tests, some tests fail. Before I got a patron with a guarantorid=112 (I let you do the math). Test plan: The easiest is to trust me Todo: Add a FK!
Created attachment 58006 [details] [review] [SIGNED OFF] Bug 17733: Fix Members.t In some dirty DB (Jenkins), the borrowers.guarantorid field is not set to a valid patron id. That make some tests fail if we create a patron with that patron id. In my DB, auto increment for borrowers is set to 52 before the tests On the 4th run of the tests, some tests fail. Before I got a patron with a guarantorid=112 (I let you do the math). Test plan: The easiest is to trust me Todo: Add a FK! Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
We need this to make Jenkins go green! :)
(In reply to Jonathan Druart from comment #1) > Test plan: > The easiest is to trust me Best plan ever?
Hmm. I manipulated guarantorid of some borrower (with your calculation ;) And the first time I ran the test, it failed: not ok 65 - TrackLastPatronActivity - 2 patrons must be deleted # Failed test 'TrackLastPatronActivity - 2 patrons must be deleted' # at t/db_dependent/Members.t line 363. # got: '1' # expected: '2' not ok 66 - TrackLastPatronActivity - 3 patrons must be deleted # Failed test 'TrackLastPatronActivity - 3 patrons must be deleted' # at t/db_dependent/Members.t line 365. # got: '2' # expected: '3' As expected, the test did not fail when running it again. So you need to remove some guarantorids here too. GetBorrowersToExpunge checks for this column.
This is something different. I would not mind fixing it in the same patch (especially to avoid yet another bug report), but I do not recreate it and so it's hard to fix :) Otherwise we could just set all the guarantorid to null Koha::Patrons->search->update( { guarantorid => null } ); That should do the job.
(In reply to Jonathan Druart from comment #6) > This is something different. > I would not mind fixing it in the same patch (especially to avoid yet > another bug report), but I do not recreate it and so it's hard to fix :) > > Otherwise we could just set all the guarantorid to null > Koha::Patrons->search->update( { guarantorid => null } ); > > That should do the job. Would not mind doing so outside sections where guarantorid is relevant.
Moving the problem from comment5 to a new report: bug 17759
Created attachment 58111 [details] [review] Bug 17733: Fix Members.t In some dirty DB (Jenkins), the borrowers.guarantorid field is not set to a valid patron id. That make some tests fail if we create a patron with that patron id. In my DB, auto increment for borrowers is set to 52 before the tests On the 4th run of the tests, some tests fail. Before I got a patron with a guarantorid=112 (I let you do the math). Test plan: The easiest is to trust me Todo: Add a FK! Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Opened a new report for special case: Bug 17759.
Pushed to master for 17.05, thanks Jonathan!
This patch has been pushed to 16.11.x, will be in 16.11.01.