Added optional dependency, so as to explain why testing explodes when the Enhanced Content system preference TagsExternalDictionary is set. Since INIT runs after BEGIN, moved INIT logic into BEGIN in order to remove INIT and thus eliminate the warning. TEST PLAN --------- 1) prove t/00-load.t -- warning about INIT for C4::Tags 2) prove `git grep -l Tags | 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 51958 [details] [review] Bug 16637: t/00-load.t warning from C4/Tags.pm Added optional dependency, so as to explain why testing explodes when the Enhanced Content system preference TagsExternalDictionary is set (/usr/bin/ispell). Since INIT runs after BEGIN, moved INIT logic into BEGIN in order to remove INIT and thus eliminate the warning. TEST PLAN --------- 1) prove t/00-load.t -- warning about INIT for C4::Tags 2) prove `git grep -l Tags | 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
Is it a duplicate of bug 16455?
(In reply to Jonathan Druart from comment #2) > Is it a duplicate of bug 16455? Not quite. Because I also expressly add the PerlDependencies.pm which helps people realize things that could explode. I'll rework this to exclude the bug 16455 portion and be dependent on it.
(In reply to M. Tompsett from comment #3) > (In reply to Jonathan Druart from comment #2) > > Is it a duplicate of bug 16455? > > Not quite. Because I also expressly add the PerlDependencies.pm which helps > people realize things that could explode. I'll rework this to exclude the > bug 16455 portion and be dependent on it. I passed QA already on 16455. Please adjust this patch?
(In reply to Marcel de Rooy from comment #4) > I passed QA already on 16455. > Please adjust this patch? I will be, but you'll notice I can't cleanly apply 16455. :)
Created attachment 52059 [details] [review] Bug 16637: Optional Dependency not listed If a user decides to set the Enhanced Content system preference TagsExternalDictionary to something (e.g. /usr/bin/ispell), but the Lingua::Ispell perl library is not installed, Koha dies ungracefully. TEST PLAN --------- 1) fill in TagsExternalDictionary system preference 2) check out the Perl modules tab in the about Koha page -- Lingua::Ispell is not listed. 3) sudo apt-get install liblingua-ispell-perl -- likely uninstalled, so this will ensure it is. 4) prove t/db_dependent/Tags.t -- should work fine. 5) sudo apt-get remove liblingua-ispell-perl 6) prove t/db_dependent/Tags.t -- it should die horribly because Lingua::Ispell is not installed. 7) apply patch 8) prove t/db_dependent/Tags.t -- like bug 16582 and the others, all tests will be skipped and a notice given of the missing optional library 9) sudo apt-get install liblingua-ispell-perl 10) prove t/db_dependent/Tags.t -- should run fine 11) check out the Perl modules tab in the about Koha page -- Lingua::Ispell is listed. 12) run koha qa test tools
This bug was initially triggered by what has been resolved in bug 16455. However, other minor side pieces were not dealt with in that bug, so this bug has been repurposed. The test plan in comment #6 reflects this.
Mark, What about this (without your patch): If I clear the externaldict pref, the module Tags compiles ok even without the Ispell module. If I enter a value in the pref, I have the Cant locate Ispell module error. Sounds weird.. You skip testing now if the module is not installed. That is not taking into consideration the value of the pref, as above.
(In reply to M. Tompsett from comment #5) > (In reply to Marcel de Rooy from comment #4) > > I passed QA already on 16455. > > Please adjust this patch? > > I will be, but you'll notice I can't cleanly apply 16455. :) Did you git fetch or apply the dependency of 16455..
Oh... right... Shoot... tweaking still...
Created attachment 52060 [details] [review] Bug 16637: Dependency for C4::Tags not listed Added optional dependency, so as to explain why testing explodes when the Enhanced Content system preference TagsExternalDictionary is set. It is optional, because not only does TagsExternalDictionary have to be set, but TagsEnabled must be 'Allow'. Also tweaked C4/Tags.pm to ignore TagsExternalDictionary, if Lingua::Ispell is not installed. A warning is given. TEST PLAN --------- 1) Set the Enhanced Content system preference TagsExternalDictionary to /usr/bin/ispell 2) sudo apt-get install liblingua-ispell-perl -- should be a new install 3) prove t/db_dependent/Tags.t -- should work fine 4) sudo apt-get remove liblingua-ispell-perl 5) prove t/db_dependent/Tags.t -- should explode 6) Clear the Enhanced Content system preference TagsExternalDictionary 7) prove t/db_dependent/Tags.t -- should work fine 8) apply patch 9) prove t/db_dependent/Tags.t -- should work fine 10) Set the Enhanced Content system preference TagsExternalDictionary to /usr/bin/ispell 11) prove t/db_dependent/Tags.t -- should work, with warning. 12) sudo apt-get install liblingua-ispell-perl 13) prove t/db_dependent/Tags.t -- should work fine 14) run koha qa test tools.
Created attachment 52061 [details] [review] Bug 16637: Dependency for C4::Tags not listed Added optional dependency, so as to explain why testing explodes when the Enhanced Content system preference TagsExternalDictionary is set. It is optional, because not only does TagsExternalDictionary have to be set, but TagsEnabled must be 'Allow'. Also tweaked C4/Tags.pm to ignore TagsExternalDictionary, if Lingua::Ispell is not installed. A warning is given. TEST PLAN --------- 1) Set the Enhanced Content system preference TagsExternalDictionary to /usr/bin/ispell 2) sudo apt-get install liblingua-ispell-perl -- should be a new install 3) prove t/db_dependent/Tags.t -- should work fine 4) sudo apt-get remove liblingua-ispell-perl 5) prove t/db_dependent/Tags.t -- should explode 6) Clear the Enhanced Content system preference TagsExternalDictionary 7) prove t/db_dependent/Tags.t -- should work fine 8) apply patch 9) prove t/db_dependent/Tags.t -- should work fine 10) Set the Enhanced Content system preference TagsExternalDictionary to /usr/bin/ispell 11) prove t/db_dependent/Tags.t -- should work, with warning. 12) sudo apt-get install liblingua-ispell-perl 13) prove t/db_dependent/Tags.t -- should work fine 14) run koha qa test tools. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Post-hackfest hotel Olympia lobby signoff. Kalimera! Works as expected. At this moment the Tags.t test does not need the database btw, but the module should have much more test coverage.
Created attachment 52100 [details] [review] Bug 16637: Dependency for C4::Tags not listed Added optional dependency, so as to explain why testing explodes when the Enhanced Content system preference TagsExternalDictionary is set. It is optional, because not only does TagsExternalDictionary have to be set, but TagsEnabled must be 'Allow'. Also tweaked C4/Tags.pm to ignore TagsExternalDictionary, if Lingua::Ispell is not installed. A warning is given. TEST PLAN --------- 1) Set the Enhanced Content system preference TagsExternalDictionary to /usr/bin/ispell 2) sudo apt-get install liblingua-ispell-perl -- should be a new install 3) prove t/db_dependent/Tags.t -- should work fine 4) sudo apt-get remove liblingua-ispell-perl 5) prove t/db_dependent/Tags.t -- should explode 6) Clear the Enhanced Content system preference TagsExternalDictionary 7) prove t/db_dependent/Tags.t -- should work fine 8) apply patch 9) prove t/db_dependent/Tags.t -- should work fine 10) Set the Enhanced Content system preference TagsExternalDictionary to /usr/bin/ispell 11) prove t/db_dependent/Tags.t -- should work, with warning. 12) sudo apt-get install liblingua-ispell-perl 13) prove t/db_dependent/Tags.t -- should work fine 14) run koha qa test tools. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Post-hackfest hotel Olympia lobby signoff. Kalimera! Works as expected. At this moment the Tags.t test does not need the database btw, but the module should have much more test coverage. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for Koha 16.11, thanks Mark!
Pushed in 16.05. Will be in 16.05.01.
Patch pushed to 3.22.x, will be in 3.22.8