Even though there is no need for anything stored in the database for the test, C4::Templates requires the database. The solution is to mock all database-using routines.
Created attachment 12607 [details] [review] Bug 8846: Exploded Terms test sneakily uses database Even though there is no need for anything stored in the database for the test, C4::Templates requires the database. The solution is to mock all database-using routines. To test: 1) Stop MySQL 2) Run test
Created attachment 12617 [details] [review] Bug 8846: Exploded Terms test sneakily uses database Even though there is no need for anything stored in the database for the test, C4::Templates requires the database and a koha-conf.xml. The solution is to mock all database- and koha-conf-using routines. To test: 1) Stop MySQL 2) Unset KOHA_CONF 3) Run test
Created attachment 12710 [details] [review] Bug 8846: Exploded Terms test sneakily uses database Even though there is no need for anything stored in the database for the test, C4::Templates requires the database and a koha-conf.xml. The solution is to mock all database- and koha-conf-using routines. To test: 1) Stop MySQL 2) Unset KOHA_CONF 3) Run test Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Followed test plan, everything checks out.
QA comment: wouldn't it be better to put Mocking subs in a specific place where they could be reused ? Mocking C4::Languages will probably be very useful for other tests There is already t/lib/Mocks/Context.pm that mocks (very partially) C4::Context isn't it a direction we should follow ?
(In reply to comment #4) > QA comment: wouldn't it be better to put Mocking subs in a specific place > where they could be reused ? Mocking C4::Languages will probably be very > useful for other tests > There is already t/lib/Mocks/Context.pm that mocks (very partially) > C4::Context > > isn't it a direction we should follow ? I think we definitely need to make it reusable, and my plan is to build in t/lib/Mocks, but I think first we need a better sense of what we're mocking where. Different sysprefs need to be mocked to different values for different tests, and I'd rather have a working test that requires a little bit of refactoring than a non-working test (i.e. one that does not actually test what it is supposed to). This is going to be one of my goals for 3.12.
So, marked as Passed QA.
mmm... trying to test this patch before pushing, I have a problem 17:18 ~/koha.dev/koha-community (new/bug_8846 $%)$ perl t/db_dependent/SuggestionEngine_ExplodedTerms.t ok 1 - use Koha::SuggestionEngine; ok 2 - Created suggestion engine Template process failed: file error - /home/paul/koha.dev/koha-community/t/db_dependent/../koha-tmpl/opac-tmpl/prog/en/modules/text/explodedterms.tt: not found at /home/paul/koha.dev/koha-community//C4/Templates.pm line 132. # Tests were run but no plan was declared and done_testing() was not seen. where's the mistake ?
(In reply to comment #7) > mmm... trying to test this patch before pushing, I have a problem > > 17:18 ~/koha.dev/koha-community (new/bug_8846 $%)$ perl > t/db_dependent/SuggestionEngine_ExplodedTerms.t > ok 1 - use Koha::SuggestionEngine; > ok 2 - Created suggestion engine > Template process failed: file error - > /home/paul/koha.dev/koha-community/t/db_dependent/../koha-tmpl/opac-tmpl/ > prog/en/modules/text/explodedterms.tt: not found at > /home/paul/koha.dev/koha-community//C4/Templates.pm line 132. > # Tests were run but no plan was declared and done_testing() was not seen. > > > where's the mistake ? The patch moving this test to db_dependent shouldn't have been pushed. This test file must be in t/
(In reply to comment #8) > (In reply to comment #7) > > mmm... trying to test this patch before pushing, I have a problem > > > > 17:18 ~/koha.dev/koha-community (new/bug_8846 $%)$ perl > > t/db_dependent/SuggestionEngine_ExplodedTerms.t > > ok 1 - use Koha::SuggestionEngine; > > ok 2 - Created suggestion engine > > Template process failed: file error - > > /home/paul/koha.dev/koha-community/t/db_dependent/../koha-tmpl/opac-tmpl/ > > prog/en/modules/text/explodedterms.tt: not found at > > /home/paul/koha.dev/koha-community//C4/Templates.pm line 132. > > # Tests were run but no plan was declared and done_testing() was not seen. > > > > > > where's the mistake ? > > The patch moving this test to db_dependent shouldn't have been pushed. This > test file must be in t/ I've continued looking into it, and I think I may have been mistaken. It looks like the problem may be that you don't have opachtdocs set in your koha-conf.xml.
(In reply to comment #9) > I've continued looking into it, and I think I may have been mistaken. It > looks like the problem may be that you don't have opachtdocs set in your > koha-conf.xml. just checked, and I have opachtdocs in my koha-conf.xml = <opachtdocs>/home/paul/koha.dev/koha-community/koha-tmpl/opac-tmpl</opachtdocs> (which point to the correct directory, just checked
Created attachment 13157 [details] [review] Bug 8846: Exploded Terms test sneakily uses database Even though there is no need for anything stored in the database for the test, C4::Templates requires the database and a koha-conf.xml. The solution is to mock all database- and koha-conf-using routines. To test: 1) Stop MySQL 2) Unset KOHA_CONF 3) Run test Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 13158 [details] [review] Bug 8846 conflict resolution: correct path The test was moved since the original patch was written, making the revised test fail. This patch corrects the mocked template patch so that the test will pass. To test: 1) Before applying patch, run `prove t/db_dependent/SuggestionEngine_ExplodedTerms.t` 2) Note that it fails. 3) Apply patch. 4) Run `prove t/db_dependent/SuggestionEngine_ExplodedTerms.t` 5) Note that it passes. 6) Celebrate.
Created attachment 13160 [details] [review] Bug 8846 conflict resolution: correct path The test was moved since the original patch was written, making the revised test fail. This patch corrects the mocked template patch so that the test will pass. To test: 1) Before applying patch, run `prove t/db_dependent/SuggestionEngine_ExplodedTerms.t` 2) Note that it fails. 3) Apply patch. 4) Run `prove t/db_dependent/SuggestionEngine_ExplodedTerms.t` 5) Note that it passes. 6) Celebrate. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
The test fails unless BOTH patches are pushed, I have signed off the second one, putting it back to signed off for QA
(In reply to comment #14) > The test fails unless BOTH patches are pushed, I have signed off the second > one, putting it back to signed off for QA test works as stated - passing QA on 2 patches... (awesome stuff Jared) $ sudo /etc/init.d/mysql stop $ export KOHA_CONF='' $ perl t/db_dependent/SuggestionEngine_ExplodedTerms.t ok 1 - use Koha::SuggestionEngine; unable to locate Koha configuration file koha-conf.xml at /home/mason/git/head/C4/Context.pm line 360. unable to locate Koha configuration file koha-conf.xml at /home/mason/git/head/C4/Context.pm line 360. Use of uninitialized value in concatenation (.) or string at /home/mason/git/head/C4/Templates.pm line 208. ok 2 - Created suggestion engine ok 3 - Suggested correct alternatives for keyword search 'Cookery' ok 4 - Suggested correct alternatives for subject search 'Cookery' ok 5 - No suggestions for fielded search ok 6 - No suggestions for CCL search 1..6 $ koha-qa.pl -c 2 -v 2 testing 2 commit(s) (applied to commit ea9e867) * a9729fd Bug 8846 conflict resolution: correct path t/db_dependent/SuggestionEngine_ExplodedTerms.t * 3207eb3 Bug 8846: Exploded Terms test sneakily uses database t/db_dependent/SuggestionEngine_ExplodedTerms.t * t/db_dependent/SuggestionEngine_ExplodedTerms.t OK pod OK forbidden patterns OK valid OK critic OK
This patch has been pushed to master and is suitable for backporting to 3.10.
Patch pushed to branch 3.10.x
Released in 3.10.0