Since the tests are expecting an initialize function, the initialize call was just moved outside of the INIT block. TEST PLAN --------- 1) prove t/00-load.t -- warnings about INIT for hbyymmincr 2) prove `git grep -l BakerTaylor | grep [.]t$` -- should all run okay 3) apply patch 4) repeat steps 1 and 2 -- warning should be gone, and everything else run okay 5) run koha qa test tools
Created attachment 51957 [details] [review] Bug 16636: t/00-load.t warning from C4/External/BakerTaylor.pm Since the tests are expecting an initialize function, the initialize call was just moved outside of the INIT block. TEST PLAN --------- 1) prove t/00-load.t -- warnings about INIT for hbyymmincr 2) prove `git grep -l BakerTaylor | grep [.]t$` -- should all run okay 3) apply patch 4) repeat steps 1 and 2 -- warning should be gone, and everything else run okay 5) run koha qa test tools
Created attachment 52069 [details] [review] [SIGNED-OFF] Bug 16636: t/00-load.t warning from C4/External/BakerTaylor.pm Since the tests are expecting an initialize function, the initialize call was just moved outside of the INIT block. TEST PLAN --------- 1) prove t/00-load.t -- warnings about INIT for hbyymmincr 2) prove `git grep -l BakerTaylor | grep [.]t$` -- should all run okay 3) apply patch 4) repeat steps 1 and 2 -- warning should be gone, and everything else run okay 5) run koha qa test tools Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Srdjan, do you forget to change bug status?
I followed the test plan and got the following. Before applying the patch, prove t/00-load.t would output the following t/00-load.t .. 1/? Too late to run INIT block at /var/koha/Koha/C4/Barcodes/hbyymmincr.pm line 41. Too late to run INIT block at /var/koha/Koha/C4/External/BakerTaylor.pm line 42. After applying the patch, it would still output 1 warning while the comment appears to indicate there should be no warnings. t/00-load.t .. 7/? Too late to run INIT block at /var/koha/Koha/C4/Barcodes/hbyymmincr.pm line 41. t/00-load.t .. ok All tests successful. Since the patch focuses on warning from BakerTaylor.pm, I assume it achieves its goal, maybe then the test plan needs to be updated.
(In reply to Florent Mara from comment #4) > I followed the test plan and got the following. > > Before applying the patch, prove t/00-load.t would output the following > t/00-load.t .. 1/? Too late to run INIT block at > /var/koha/Koha/C4/Barcodes/hbyymmincr.pm line 41. > Too late to run INIT block at /var/koha/Koha/C4/External/BakerTaylor.pm line > 42. > > After applying the patch, it would still output 1 warning while the comment > appears to indicate there should be no warnings. > t/00-load.t .. 7/? Too late to run INIT block at > /var/koha/Koha/C4/Barcodes/hbyymmincr.pm line 41. > t/00-load.t .. ok > All tests successful. > > Since the patch focuses on warning from BakerTaylor.pm, I assume it achieves > its goal, maybe then the test plan needs to be updated. Yes, the test plan was a cut-and-paste mistake by me. It should be TEST PLAN --------- 1) prove t/00-load.t -- warnings about INIT for BakerTaylor 2) prove `git grep -l BakerTaylor | grep [.]t$` -- should all run okay 3) apply patch 4) repeat steps 1 and 2 -- warning should be gone, and everything else run okay 5) run koha qa test tools
Then it is good for me. :-) The patch behaves as described in the test plan and fixes the warning from BakerTaylor.pm
Created attachment 52104 [details] [review] [SIGNED-OFF] Bug 16636: t/00-load.t warning from C4/External/BakerTaylor.pm Since the tests are expecting an initialize function, the initialize call was just moved outside of the INIT block. TEST PLAN --------- 1) prove t/00-load.t -- warnings about INIT for BakerTaylor 2) prove `git grep -l BakerTaylor | grep [.]t$` -- should all run okay 3) apply patch 4) repeat steps 1 and 2 -- warning should be gone, and everything else run okay 5) run koha qa test tools Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Florent Mara <florent.mara@gmail.com> NOTE: Tweaked test plan based on comment #4, Added sign off based on comment #6. Current status: Signed Off
We will get rid of the warning, but the it won't make it works correctly under Plack. I may be better to keep the warning to tell something is wrong in this area and need to be fixed.
Created attachment 52213 [details] [review] Bug 16636: t/00-load.t warning from C4/External/BakerTaylor.pm Since the tests are expecting an initialize function, the initialize call was just moved outside of the INIT block. TEST PLAN --------- 1) prove t/00-load.t -- warnings about INIT for BakerTaylor 2) prove `git grep -l BakerTaylor | grep [.]t$` -- should all run okay 3) apply patch 4) repeat steps 1 and 2 -- warning should be gone, and everything else run okay 5) run koha qa test tools Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Florent Mara <florent.mara@gmail.com> NOTE: Tweaked test plan based on comment #4, Added sign off based on comment #6. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 52214 [details] [review] Bug 16636: [QA Follow-up] Make BakerTaylor plack safe Initialize file level lexicals each call. Do not call _initialize outside the module. Adjust test by mocking preferences. Test plan: Run t/db_dependent/External_BakerTaylor.t. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested module with trivial script under Plack/memcached by toggling the associated preferences.
Created attachment 52215 [details] [review] Bug 16636: [QA Follow-up] Move External_BakerTaylor.t to t When we mock the preferences completely now, we do not need db access anymore. Test plan: Run the test without exporting KOHA_CONF. The test passes with some warns about koha-conf.xml. If you point KOHA_CONF to etc/koha-conf.xml, you have no warns. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 52253 [details] [review] Bug 16636 [QA Followup] - Move tests to t/External
Pushed to master for 16.11, thanks Mark, Marcell!
Pushed in 16.05. Will be in 16.05.01.
Patch pushed to 3.22.x, will be in 3.22.8
*** Bug 16342 has been marked as a duplicate of this bug. ***