|
Description
Lee Jamison
2017-07-05 17:48:37 UTC
Created attachment 64818 [details] [review] Bug 18897 - Koha_Elasticsearch_Indexer.t doesn't skip when ES module not installed If Catmandu::Importer::MARC is not installed, Koha_Elasticsearch_Indexer.t fails instead of skipping. Test Plan: 1. Run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" as koha-shell. 2. Note failure on test 1/5 indicating module not installed. 3. Apply patch. 4. Re-run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" 5. Note test now skips. Created attachment 66478 [details] [review] Bug 18897: Koha_Elasticsearch_Indexer.t doesn't skip when ES module(s) not installed If Catmandu::Importer::MARC is not installed, Koha_Elasticsearch_Indexer.t fails instead of skipping. It should skip as ES is not currently mandatory. Test Plan: 1. Run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" as koha-shell 2. Note failure on test 1/5 indicating module not installed. 3. Apply patch. 4. Re-run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" 5. Note test now skips. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 66479 [details] [review] Bug 18897: Improve error message Test Plan: $ git fetch $ git checkout -b bug_18897 origin/master $ git bz apply 18897 -- interactive apply just the first patch $ sudo apt-get remove libcatmandu-store-elasticsearch-perl ... $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the wrong library is missing. $ git bz apply 18897 -- interactive apply this patch $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the correct library is missing. run the koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 66568 [details] [review] Bug 18897: Koha_Elasticsearch_Indexer.t doesn't skip when ES module(s) not installed If Catmandu::Importer::MARC is not installed, Koha_Elasticsearch_Indexer.t fails instead of skipping. It should skip as ES is not currently mandatory. Test Plan: 1. Run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" as koha-shell 2. Note failure on test 1/5 indicating module not installed. 3. Apply patch. 4. Re-run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" 5. Note test now skips. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Created attachment 66569 [details] [review] Bug 18897: Improve error message Test Plan: $ git fetch $ git checkout -b bug_18897 origin/master $ git bz apply 18897 -- interactive apply just the first patch $ sudo apt-get remove libcatmandu-store-elasticsearch-perl ... $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the wrong library is missing. $ git bz apply 18897 -- interactive apply this patch $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the correct library is missing. run the koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Lee Jamison <ldjamison@marywood.edu> It would be better to use Module::Load::Conditional::can_load I think Failing per comment #6 Created attachment 66777 [details] [review] Bug 18897: Use Module::Load::Conditional::can_load Rather than rely on eval(), Jonathan Druart recommended in comment #6 to use Use Module::Load::Conditional::can_load. This addresses that concern. TEST PLAN --------- $ git fetch $ git checkout -b bug_18897 origin/master $ git bz apply 18897 $ sudo apt-get remove libcatmandu-store-elasticsearch-perl ... $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the correct library is missing. $ sudo apt-get install libcatmandu-store-elasticsearch-perl -- Note it runs as expected. run the koha qa test tools Missed a line:
> TEST PLAN
> ---------
> $ git fetch
> $ git checkout -b bug_18897 origin/master
> $ git bz apply 18897
> $ sudo apt-get remove libcatmandu-store-elasticsearch-perl
> ...
> $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t
> -- Note that the message says the correct library is missing.
> $ sudo apt-get install libcatmandu-store-elasticsearch-perl
> $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t
> -- Note it runs as expected.
> run the koha qa test tools
Created attachment 66779 [details] [review] Bug 18897: Use Module::Load::Conditional::can_load Rather than rely on eval(), Jonathan Druart recommended in comment #6 to use Use Module::Load::Conditional::can_load. This addresses that concern. TEST PLAN --------- $ git fetch $ git checkout -b bug_18897 origin/master $ git bz apply 18897 $ sudo apt-get remove libcatmandu-store-elasticsearch-perl ... $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the correct library is missing. $ sudo apt-get install libcatmandu-store-elasticsearch-perl -- Note it runs as expected. run the koha qa test tools Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Created attachment 67521 [details] [review] Bug 18897: Koha_Elasticsearch_Indexer.t doesn't skip when ES module(s) not installed If Catmandu::Importer::MARC is not installed, Koha_Elasticsearch_Indexer.t fails instead of skipping. It should skip as ES is not currently mandatory. Test Plan: 1. Run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" as koha-shell 2. Note failure on test 1/5 indicating module not installed. 3. Apply patch. 4. Re-run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" 5. Note test now skips. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67522 [details] [review] Bug 18897: Improve error message Test Plan: $ git fetch $ git checkout -b bug_18897 origin/master $ git bz apply 18897 -- interactive apply just the first patch $ sudo apt-get remove libcatmandu-store-elasticsearch-perl ... $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the wrong library is missing. $ git bz apply 18897 -- interactive apply this patch $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the correct library is missing. run the koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67523 [details] [review] Bug 18897: Use Module::Load::Conditional::can_load Rather than rely on eval(), Jonathan Druart recommended in comment #6 to use Use Module::Load::Conditional::can_load. This addresses that concern. TEST PLAN --------- $ git fetch $ git checkout -b bug_18897 origin/master $ git bz apply 18897 $ sudo apt-get remove libcatmandu-store-elasticsearch-perl ... $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the correct library is missing. $ sudo apt-get install libcatmandu-store-elasticsearch-perl -- Note it runs as expected. run the koha qa test tools Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Sounds like we have other tests failing: # Failed test 'Syntax check misc/search_tools/rebuild_elastic_search.pl' # at /usr/share/perl5/Test/Strict.pm line 406. # Can't locate Catmandu/Importer/MARC.pm in @INC (you may need to install the Catmandu::Importer::MARC module) (@INC contains: installer misc/translator /kohadevbox/koha /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 /usr/local/share/perl/5.20.2/x86_64-linux-gnu-thread-multi /usr/local/share/perl/5.20.2 /usr/lib/x86_64-linux-gnu/perl5/5.20 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 /usr/local/lib/site_perl . /kohadevbox/koha /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 /usr/local/share/perl/5.20.2 /usr/lib/x86_64-linux-gnu/perl5/5.20 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 /usr/local/lib/site_perl .) at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Indexer.pm line 26. # BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Indexer.pm line 26. # Compilation failed in require at misc/search_tools/rebuild_elastic_search.pl line 89. # BEGIN failed--compilation aborted at misc/search_tools/rebuild_elastic_search.pl line 89. # Looks like you failed 1 test of 663. [23:53:44] t/db_dependent/00-strict.t # Failed test 'use Koha::SearchEngine::Elasticsearch::Search;' # at t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 27. # Tried to use 'Koha::SearchEngine::Elasticsearch::Search'. # Error: Can't locate Catmandu/Store/ElasticSearch.pm in @INC (you may need to install the Catmandu::Store::ElasticSearch module) (@INC contains: /kohadevbox/koha /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 /usr/local/share/perl/5.20.2 /usr/lib/x86_64-linux-gnu/perl5/5.20 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 /usr/local/lib/site_perl .) at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Search.pm line 49. # BEGIN failed--compilation aborted at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Search.pm line 49. # Compilation failed in require at t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 27. # BEGIN failed--compilation aborted at t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 27. Can't locate object method "search" via package "Koha::SearchEngine::Elasticsearch::Search" at t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 47. # Looks like your test exited with 255 just after 4. [22:48:34] t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t (In reply to Jonathan Druart from comment #14) > Sounds like we have other tests failing: > > # Failed test 'Syntax check misc/search_tools/rebuild_elastic_search.pl' > # at /usr/share/perl5/Test/Strict.pm line 406. > # Can't locate Catmandu/Importer/MARC.pm in @INC (you may need to install > the Catmandu::Importer::MARC module) (@INC contains: installer > misc/translator /kohadevbox/koha /etc/perl > /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 > /usr/local/share/perl/5.20.2/x86_64-linux-gnu-thread-multi > /usr/local/share/perl/5.20.2 /usr/lib/x86_64-linux-gnu/perl5/5.20 > /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 > /usr/local/lib/site_perl . /kohadevbox/koha /etc/perl > /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 /usr/local/share/perl/5.20.2 > /usr/lib/x86_64-linux-gnu/perl5/5.20 /usr/share/perl5 > /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 > /usr/local/lib/site_perl .) at > /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Indexer.pm line 26. > # BEGIN failed--compilation aborted at > /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Indexer.pm line 26. > # Compilation failed in require at > misc/search_tools/rebuild_elastic_search.pl line 89. > # BEGIN failed--compilation aborted at > misc/search_tools/rebuild_elastic_search.pl line 89. > # Looks like you failed 1 test of 663. > [23:53:44] t/db_dependent/00-strict.t > > > > # Failed test 'use Koha::SearchEngine::Elasticsearch::Search;' > # at t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 27. > # Tried to use 'Koha::SearchEngine::Elasticsearch::Search'. > # Error: Can't locate Catmandu/Store/ElasticSearch.pm in @INC (you may > need to install the Catmandu::Store::ElasticSearch module) (@INC contains: > /kohadevbox/koha /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 > /usr/local/share/perl/5.20.2 /usr/lib/x86_64-linux-gnu/perl5/5.20 > /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 > /usr/local/lib/site_perl .) at > /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Search.pm line 49. > # BEGIN failed--compilation aborted at > /kohadevbox/koha/Koha/SearchEngine/Elasticsearch/Search.pm line 49. > # Compilation failed in require at > t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 27. > # BEGIN failed--compilation aborted at > t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 27. > Can't locate object method "search" via package > "Koha::SearchEngine::Elasticsearch::Search" at > t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 47. > # Looks like your test exited with 255 just after 4. > [22:48:34] t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t I would argue that OTHER failures are beyond the scope of this patch, and that it should go back to passed QA, and two more bugs created. (In reply to M. Tompsett from comment #15) > I would argue that OTHER failures are beyond the scope of this patch, and > that it should go back to passed QA, and two more bugs created. I do not think so, the fix should be quite trivial. Created attachment 67539 [details] [review] Bug 18897: Skipping t/db_dependend/00-strict elastic search stuffs as needed Test Plan: $ git fetch $ git checkout -b bug_18897 origin/master $ sudo apt-get remove libcatmandu-store-elasticsearch-perl $ prove t/db_dependent/00-strict.t -- nasty failure. $ git bz apply 18897 -- interactive apply just the four patchs $ prove t/db_dependent/00-strict.t -- nicer skip message run the koha qa test tools Created attachment 67540 [details] [review] Bug 18897: Add skipping to t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t Test Plan: $ git fetch $ git checkout -b bug_18897 origin/master $ sudo apt-get remove libcatmandu-store-elasticsearch-perl $ prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -- nasty failure. $ git bz apply 18897 -- apply all the patches $ prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -- nicer skip message run the koha qa test tools This should pass QA quick enough, I hope. Created attachment 67553 [details] [review] Bug 18897: Koha_Elasticsearch_Indexer.t doesn't skip when ES module(s) not installed If Catmandu::Importer::MARC is not installed, Koha_Elasticsearch_Indexer.t fails instead of skipping. It should skip as ES is not currently mandatory. Test Plan: 1. Run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" as koha-shell 2. Note failure on test 1/5 indicating module not installed. 3. Apply patch. 4. Re-run "prove t/db_dependent/Koha_Elasticsearch_Indexer.t" 5. Note test now skips. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67554 [details] [review] Bug 18897: Improve error message Test Plan: $ git fetch $ git checkout -b bug_18897 origin/master $ git bz apply 18897 -- interactive apply just the first patch $ sudo apt-get remove libcatmandu-store-elasticsearch-perl ... $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the wrong library is missing. $ git bz apply 18897 -- interactive apply this patch $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the correct library is missing. run the koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67555 [details] [review] Bug 18897: Use Module::Load::Conditional::can_load Rather than rely on eval(), Jonathan Druart recommended in comment #6 to use Use Module::Load::Conditional::can_load. This addresses that concern. TEST PLAN --------- $ git fetch $ git checkout -b bug_18897 origin/master $ git bz apply 18897 $ sudo apt-get remove libcatmandu-store-elasticsearch-perl ... $ prove t/db_dependent/Koha_Elasticsearch_Indexer.t -- Note that the message says the correct library is missing. $ sudo apt-get install libcatmandu-store-elasticsearch-perl -- Note it runs as expected. run the koha qa test tools Signed-off-by: Lee Jamison <ldjamison@marywood.edu> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67556 [details] [review] Bug 18897: Skipping t/db_dependend/00-strict elastic search stuffs as needed Test Plan: $ git fetch $ git checkout -b bug_18897 origin/master $ sudo apt-get remove libcatmandu-store-elasticsearch-perl $ prove t/db_dependent/00-strict.t -- nasty failure. $ git bz apply 18897 -- interactive apply just the four patchs $ prove t/db_dependent/00-strict.t -- nicer skip message run the koha qa test tools Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67557 [details] [review] Bug 18897: Add skipping to t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t Test Plan: $ git fetch $ git checkout -b bug_18897 origin/master $ sudo apt-get remove libcatmandu-store-elasticsearch-perl $ prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -- nasty failure. $ git bz apply 18897 -- apply all the patches $ prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -- nicer skip message run the koha qa test tools Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> The patches work as expected. I wouldn't recommend them for the master branch, though. The patches are needed on the stable branches only because the dependencies are not correctly pulled. This is not that problematic on the stable branches, because ES is not ready. But master (and 17.11) should definitely pull the right dependencies so more people use and test the ES integration. Pushed to 17.05.x, will be in 17.05.05. Thanks everyone for your job, Jenkins is about to turn green ;) (In reply to Tomás Cohen Arazi from comment #25) > The patches work as expected. I wouldn't recommend them for the master > branch, though. I agree. If you push these to master, making sure the right dependencies are pulled for 17.11 and later will be missed. And as ES is maturing, and needs to be used and tested in 17.11, errors will force people to make sure the dependencies are right. Pushing these to master will give nice skipping messages and people may forget more easily. These patches have been pushed to 16.11.x and will be in 16.11.13. |