Bug 8353

Summary: Add test to compare system preferences in db to syspref.sql
Product: Koha Reporter: Jared Camins-Esakov <jcamins>
Component: Test SuiteAssignee: laurenthdl <henridamien.laurent>
Status: CLOSED FIXED QA Contact: Paul Poulain <paul.poulain>
Severity: enhancement    
Priority: P5 - low CC: chris, paul.poulain
Version: 3.10   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 8353: Adds a test to check for missing sysprefs in the database
Bug 8353 follow-up, perltidy
Bug 8353 follow-up adding a tiny sh in misc/maintenance
Bug 8353: Adds a test to check for missing sysprefs in the database
Bug 8353 follow-up: Perltidy new test
Bug 8353 follow-up adding a tiny sh in misc/maintenance

Description Jared Camins-Esakov 2012-07-03 16:22:43 UTC
To test:
1. Run prove xt/check_sysprefs.t
2. Note that the results warn you if any sysprefs are missing
Comment 1 Jared Camins-Esakov 2012-07-03 16:31:14 UTC Comment hidden (obsolete)
Comment 2 Paul Poulain 2012-07-10 14:33:57 UTC Comment hidden (obsolete)
Comment 3 Paul Poulain 2012-07-10 14:34:17 UTC
QA comment: new script, usefull for testing system preferences. well documented, properly written. I just submit a follow-up with the script perltidy-ed

Chris = before pushing, could you confirm it should be in xt/ ? It's a db dependant test, that is useless to run on jenkins, because -unless there's something really weird- the table will always contain sysprefs.sql because it has been filed from there !
Comment 4 Chris Cormack 2012-07-11 06:40:37 UTC
xt = release tests 

so yeah, this probably better in t/db_dependent
Comment 5 Paul Poulain 2012-07-12 16:16:20 UTC
(In reply to comment #4)
> xt = release tests 
> 
> so yeah, this probably better in t/db_dependent

Thinking of it I think it's more a maintainance script, that should be in misc/maintenance than a test for jenkins: This is usefull for a production server, to check if something is missing in your systempreferences.

Chris, do you agree ? If yes, i'll write a follow-up to move the script to misc/maintenance
Comment 6 Chris Cormack 2012-07-12 22:04:28 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > xt = release tests 
> > 
> > so yeah, this probably better in t/db_dependent
> 
> Thinking of it I think it's more a maintainance script, that should be in
> misc/maintenance than a test for jenkins: This is usefull for a production
> server, to check if something is missing in your systempreferences.
> 
> Chris, do you agree ? If yes, i'll write a follow-up to move the script to
> misc/maintenance

I think maintenance would be fine, but it's Jared's patch i'll defer to him.
Comment 7 Jared Camins-Esakov 2012-07-12 22:29:53 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Thinking of it I think it's more a maintainance script, that should be in
> > misc/maintenance than a test for jenkins: This is usefull for a production
> > server, to check if something is missing in your systempreferences.
> > 
> > Chris, do you agree ? If yes, i'll write a follow-up to move the script to
> > misc/maintenance
> 
> I think maintenance would be fine, but it's Jared's patch i'll defer to him.

I would greatly prefer it be in t/db_dependent. As of right now, we do practically nothing to test upgrades (in terms of automated testing, at least). Although the test would be essentially a no-op in a new installation, it would be very useful in testing an upgrade.
Comment 8 Paul Poulain 2012-07-13 09:53:55 UTC Comment hidden (obsolete)
Comment 9 Paul Poulain 2012-07-13 09:55:57 UTC
(In reply to comment #7)
> > > Chris, do you agree ? If yes, i'll write a follow-up to move the script to
> > > misc/maintenance
> > 
> > I think maintenance would be fine, but it's Jared's patch i'll defer to him.
> 
> I would greatly prefer it be in t/db_dependent. As of right now, we do
> practically nothing to test upgrades (in terms of automated testing, at
> least). Although the test would be essentially a no-op in a new
> installation, it would be very useful in testing an upgrade.

In french we say "cheese and desert". With the small 2nd follow-up, we add a tiny sh script that run the test.
Sysadmin look in misc/maintenance, they won't necessarly know there's something usefull in xt/ for daily maintenance. With this 4 lines sh, they will.
(I've added the --showsql option as a sysadmin want to know what to add)

Please signoff & I'll push
Comment 10 Jared Camins-Esakov 2012-07-13 12:12:10 UTC
Created attachment 10814 [details] [review]
Bug 8353: Adds a test to check for missing sysprefs in the database

Adds a non-test mode (--showsql option) that displays the missing
sysprefs as sql inserts.  Some sysprefs were missed from the syspref
file due to extra spaces in the insert : fixed. The test now checks if
there are at least as many sysprefs in the database as in the syspref
file (instead of checking for the exact number)

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Cherrypicked from BibLibre master, rebased onto current master, and
corrected typos, 2012-07-03

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 11 Jared Camins-Esakov 2012-07-13 12:12:23 UTC
Created attachment 10815 [details] [review]
Bug 8353 follow-up: Perltidy new test

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 12 Jared Camins-Esakov 2012-07-13 12:13:10 UTC
Created attachment 10816 [details] [review]
Bug 8353 follow-up adding a tiny sh in misc/maintenance

This script will help the sysadmin know there's a test he can use
during maintenance

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 13 Paul Poulain 2012-07-13 12:45:23 UTC
Patch pushed
Comment 14 Jared Camins-Esakov 2012-12-03 13:23:30 UTC
This test exists on master.