If you have more than the 8 default itemtypes, prove t/db_dependent/Suggestions.t fails.
Created attachment 34372 [details] [review] Bug 13457: Suggestions.t expects sample itemtypes TEST PLAN --------- 1) Make sure you have more than 8 item types, and preferably something with a non-sample default code for itemtypes. 2) prove t/db_dependent/Suggestions.t -- this will fail 3) Apply patch 4) prove t/db_dependent/Suggestions.t -- this will succeed 5) run koha qa test tools
Created attachment 34429 [details] [review] Bug 13457: Suggestions.t expects sample itemtypes I've applied the patch against 3.19.00.000 Everything is OK as required. In detail of step 5) I saw: koha-qa.pl -v 2 -c 1 testing 1 commit(s) (applied to 5bdf460 'Bug 13425 - XSS in opac facets - Patc') Processing files before patches |========================>| 1 / 1 (100.00%) Processing files after patches |========================>| 1 / 1 (100.00%) OK t/db_dependent/Suggestions.t OK critic OK forbidden patterns OK pod OK valid So I pass the patch to "Signed Off" status.
The tests don't pass if you don't use the sample data. To me, this patch is not relevant: we should not create all data before launching tests. For instance the same test file (Suggestions.t) won't pass if you don't have the CPL branchcode and S categorycode.
Not sure if I understand what you mean. Wouldn't it be best if we made the UT independent of what's in the database? I think this patch is useful, as I often test with the default data, but may add new branches to it. So while it would be best to have complete data independence, this is at least a small step in the right direction. (In reply to Jonathan Druart from comment #3) > The tests don't pass if you don't use the sample data. > To me, this patch is not relevant: we should not create all data before > launching tests. > > For instance the same test file (Suggestions.t) won't pass if you don't have > the CPL branchcode and S categorycode.
Created attachment 34575 [details] [review] [PASSED QA] Bug 13457: Suggestions.t expects sample itemtypes TEST PLAN --------- 1) Make sure you have more than 8 item types, and preferably something with a non-sample default code for itemtypes. 2) prove t/db_dependent/Suggestions.t -- this will fail 3) Apply patch 4) prove t/db_dependent/Suggestions.t -- this will succeed 5) run koha qa test tools Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Just tested by deleting CPL and S. Comment #3 requires another patch. Working on it.
Created attachment 34595 [details] [review] Bug 13457 - Followup for CPL and S codes Based on comment #3, this corrects CPL and S issues if they do not exist in the DB. TEST PLAN --------- 0) Backup your DB. 1) Clear CPL and S from your DB. - delete from borrowers where categorycode='S'; - delete from categories where categorycode='S'; - delete from borrowers where branchcode='CPL'; - delete from branch where branchcode='CPL'; 2) prove t/db_dependent/Suggestions.t -- This should fail. 3) Apply patch 4) prove t/db_dependent/Suggestions.t -- This should work. 5) Intentionally add categorycode 'S' and branchcode 'CPL' back into the database. 6) prove t/db_dependent/Suggestions.t -- This should work. 7) run koha qa test tools. 8) Restore your DB :)
(In reply to Kyle M Hall from comment #4) > Not sure if I understand what you mean. Wouldn't it be best if we made the > UT independent of what's in the database? Yes, of course. > I think this patch is useful, as I > often test with the default data, but may add new branches to it. So while > it would be best to have complete data independence, this is at least a > small step in the right direction. I am not sure adding >20 lines per test file is a good idea. Especially because we will c/p the same lines. If we consider we need to create all data before writing one test, the tests will be more difficult to read than now.
Created attachment 34827 [details] [review] [PASSED QA] Bug 13457: Suggestions.t expects sample itemtypes I've applied the patches against master 3.19.00.003 head 13252. Everything worked as required. I pass the patch to "Signed Off" status. ---------------------------------------------------------------------------- This was the error expected at step 2) (RIGHT, error as expected) prove t/db_dependent/Suggestions.t t/db_dependent/Suggestions.t .. 1/97 Can't call method "default_privacy" on an undefined value at [....]/C4/Members.pm line 874. # Looks like you planned 97 tests but ran 2. # Looks like your test exited with 255 just after 2. t/db_dependent/Suggestions.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 95/97 subtests Test Summary Report ------------------- t/db_dependent/Suggestions.t (Wstat: 65280 Tests: 2 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 97 tests but ran 2. Files=1, Tests=2, 1 wallclock secs ( 0.00 usr 0.03 sys + 0.40 cusr 0.71 csys = 1.14 CPU) Result: FAIL -------------------------------------------------------- This was the test required at the 4) and 6) steps (RIGHT): prove t/db_dependent/Suggestions.t t/db_dependent/Suggestions.t .. ok All tests successful. Files=1, Tests=97, 1 wallclock secs ( 0.00 usr 0.02 sys + 0.82 cusr 0.22 csys = 1.06 CPU) Result: PASS ----------------------------------------------------------------------------
Created attachment 34828 [details] [review] Bug 13457 - Followup for CPL and S codes
Created attachment 34919 [details] [review] [PASSED QA] Bug 13457: Suggestions.t expects sample itemtypes TEST PLAN --------- 1) Make sure you have more than 8 item types, and preferably something with a non-sample default code for itemtypes. 2) prove t/db_dependent/Suggestions.t -- this will fail 3) Apply patch 4) prove t/db_dependent/Suggestions.t -- this will succeed 5) run koha qa test tools Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 34920 [details] [review] [PASSED QA] Bug 13457 - Followup for CPL and S codes Based on comment #3, this corrects CPL and S issues if they do not exist in the DB. TEST PLAN --------- 0) Backup your DB. 1) Clear CPL and S from your DB. - delete from borrowers where categorycode='S'; - delete from categories where categorycode='S'; - delete from borrowers where branchcode='CPL'; - delete from branch where branchcode='CPL'; 2) prove t/db_dependent/Suggestions.t -- This should fail. 3) Apply patch 4) prove t/db_dependent/Suggestions.t -- This should work. 5) Intentionally add categorycode 'S' and branchcode 'CPL' back into the database. 6) prove t/db_dependent/Suggestions.t -- This should work. 7) run koha qa test tools. 8) Restore your DB :) Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
It's probably not the best solution, but I think it can be easily redone, once we figured out better ways.
(In reply to Katrin Fischer from comment #13) > It's probably not the best solution, but I think it can be easily redone, > once we figured out better ways. IMO what we do now (only pass with specific data) is not perfect but better than the idea behind this patch (copy and paste the data insertion).
So you feel it adds too much data? Sorry, not sure I understand yet!
Hi Mark, it doesn't seem to apply on my master branch. Could you please take a look?
In master 3.19.00.006 t/db_dependent/Suggestions.t has 1 more line: use C4::Budgets; So the 2-nd patch would need to be rebased.
Created attachment 35254 [details] [review] [PASSED QA] Bug 13457 - Followup for CPL and S codes Based on comment #3, this corrects CPL and S issues if they do not exist in the DB. TEST PLAN --------- 0) Backup your DB. 1) Clear CPL and S from your DB. - delete from borrowers where categorycode='S'; - delete from categories where categorycode='S'; - delete from borrowers where branchcode='CPL'; - delete from branch where branchcode='CPL'; 2) prove t/db_dependent/Suggestions.t -- This should fail. 3) Apply patch 4) prove t/db_dependent/Suggestions.t -- This should work. 5) Intentionally add categorycode 'S' and branchcode 'CPL' back into the database. 6) prove t/db_dependent/Suggestions.t -- This should work. 7) run koha qa test tools. 8) Restore your DB :) Signed-off-by: Paola Rossi <paola.rossi@cineca.it> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Rebased. Paola Rossi's comment #17 was correct. Simple fix.
Patches pushed to master. Thanks Mark!
Pushed to 3.18.x will be in 3.18.4