Summary: | Code to check perl module versions is buggy | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | dcook, kyle, m.de.rooy |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 17986 | ||
Bug Blocks: | |||
Attachments: |
Bug 17990: Refactor Perl module versions check
Bug 17990: Refactor Perl module versions check Bug 17990: Refactor Perl module versions check |
Description
Jonathan Druart
2017-01-25 11:41:14 UTC
Created attachment 59546 [details] [review] Bug 17990: Refactor Perl module versions check The code is duplicated, variable are not set ($_), code is hard to read, not covered by tests and the subroutine has 2 completely different behaviors depending on the presence of the "module" parameter. No need more ti rewrite it. Test plan: - Use koha_perl_deps.pl with the different options (-u -m -a -i) - Go on the about page, "Perl modules" tab You should not see any differences from before and after this patch After looking through the master code a bit, it seems like we never actually use this form in anything but tests: $modules->version_info('module'=>"DBI"); *** Bug 17882 has been marked as a duplicate of this bug. *** *** Bug 17887 has been marked as a duplicate of this bug. *** Created attachment 59590 [details] [review] Bug 17990: Refactor Perl module versions check The code is duplicated, variable are not set ($_), code is hard to read, not covered by tests and the subroutine has 2 completely different behaviors depending on the presence of the "module" parameter. No need more ti rewrite it. Test plan: - Use koha_perl_deps.pl with the different options (-u -m -a -i) - Go on the about page, "Perl modules" tab You should not see any differences from before and after this patch Signed-off-by: David Cook <dcook@prosentient.com.au> I also ran the following tests and they all passed. perl t/Installer_PerlModules.t perl t/Installer_pm.t Created attachment 59601 [details] [review] Bug 17990: Refactor Perl module versions check The code is duplicated, variable are not set ($_), code is hard to read, not covered by tests and the subroutine has 2 completely different behaviors depending on the presence of the "module" parameter. No need more ti rewrite it. Test plan: - Use koha_perl_deps.pl with the different options (-u -m -a -i) - Go on the about page, "Perl modules" tab You should not see any differences from before and after this patch Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Good job, Jonathan. ok( $dbi_is_missing || $dbi_is_upgrade || $dbi_is_current, 'DBI should either be missing, upgrade or current' ); So now we are safe :) Passed QA Heh yes this test looks a bit silly, but it was to make sure the returned structure was still correct. To make it pass in every situation I needed to test all of them :) Pushed to master for 17.05, thanks Jonathan! Is this correctly classified as an enh? (In reply to Katrin Fischer from comment #11) > Is this correctly classified as an enh? Yes, the buggy code was not triggered. So this patch should not fixed any bugs. This won't get ported back to 16.11.x as it is an enhancement. |