Bug 8846

Summary: t/SuggestionEngine_ExplodedTerms.t sneakily uses database
Product: Koha Reporter: Jared Camins-Esakov <jcamins>
Component: Test SuiteAssignee: Jared Camins-Esakov <jcamins>
Status: CLOSED FIXED QA Contact:
Severity: blocker    
Priority: P1 - high CC: chris, jonathan.druart, kyle, mtj, paul.poulain
Version: 3.10   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16869
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 8890    
Attachments: Bug 8846: Exploded Terms test sneakily uses database
Bug 8846: Exploded Terms test sneakily uses database
Bug 8846: Exploded Terms test sneakily uses database
Bug 8846: Exploded Terms test sneakily uses database
Bug 8846 conflict resolution: correct path
Bug 8846 conflict resolution: correct path

Description Jared Camins-Esakov 2012-09-29 13:27:43 UTC
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.
Comment 1 Jared Camins-Esakov 2012-09-29 13:30:26 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2012-10-01 09:26:55 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2012-10-05 13:56:33 UTC Comment hidden (obsolete)
Comment 4 Paul Poulain 2012-10-24 14:04:39 UTC
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 ?
Comment 5 Jared Camins-Esakov 2012-10-24 14:17:56 UTC
(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.
Comment 6 Jonathan Druart 2012-10-25 14:20:18 UTC
So, marked as Passed QA.
Comment 7 Paul Poulain 2012-10-26 15:21:02 UTC
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 ?
Comment 8 Jared Camins-Esakov 2012-10-26 15:23:41 UTC
(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/
Comment 9 Jared Camins-Esakov 2012-10-26 15:33:36 UTC
(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.
Comment 10 Paul Poulain 2012-10-29 18:39:32 UTC
(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
Comment 11 Jared Camins-Esakov 2012-10-31 21:10:49 UTC Comment hidden (obsolete)
Comment 12 Jared Camins-Esakov 2012-10-31 21:11:03 UTC Comment hidden (obsolete)
Comment 13 Chris Cormack 2012-10-31 21:47:23 UTC Comment hidden (obsolete)
Comment 14 Chris Cormack 2012-10-31 21:48:25 UTC
The test fails unless BOTH patches are pushed, I have signed off the second one, putting it back to signed off for QA
Comment 15 Mason James 2012-11-01 06:41:38 UTC
(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
Comment 16 Jared Camins-Esakov 2012-11-01 14:35:12 UTC
This patch has been pushed to master and is suitable for backporting to 3.10.
Comment 17 Paul Poulain 2012-11-01 21:33:02 UTC
Patch pushed to branch 3.10.x
Comment 18 Chris Cormack 2013-04-21 10:45:55 UTC
Released in 3.10.0