Bug 8353 - Add test to compare system preferences in db to syspref.sql
Summary: Add test to compare system preferences in db to syspref.sql
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low enhancement (vote)
Assignee: laurenthdl
QA Contact: Paul Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-03 16:22 UTC by Jared Camins-Esakov
Modified: 2013-12-05 20:01 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
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 (4.53 KB, patch)
2012-07-03 16:31 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8353 follow-up, perltidy (3.79 KB, patch)
2012-07-10 14:33 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 8353 follow-up adding a tiny sh in misc/maintenance (989 bytes, patch)
2012-07-13 09:53 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 8353: Adds a test to check for missing sysprefs in the database (4.71 KB, patch)
2012-07-13 12:12 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8353 follow-up: Perltidy new test (3.91 KB, patch)
2012-07-13 12:12 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8353 follow-up adding a tiny sh in misc/maintenance (1.11 KB, patch)
2012-07-13 12:13 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.