Description
Marcel de Rooy
2017-02-02 07:54:27 UTC
Created attachment 59767 [details] [review] Bug 18036: Improve test coverage for themelanguage This test triggers the three cases that were modified. prove -v t/db_dependent/Templates.t Saved a kitten for you, Mark. But still needs some polishing.. (In reply to Marcel de Rooy from comment #2) > Saved a kitten for you, Mark. But still needs some polishing.. Thanks, I'll wait for bug 17982 to get pushed, then rebase. Created attachment 59818 [details] [review] Bug 18036: Perlcritic improve t/db_dependent/Templates.t TEST PLAN --------- 1) apply 17982 2) perlcritic -3 t/db_dependent/Templates.t -- messages 3) apply this patch 4) perlcritic -3 t/db_dependent/Templates.t -- OK 5) run koha qa test tools Created attachment 59819 [details] [review] Bug 18036: All 4 cases tested Created attachment 59820 [details] [review] Proof that all 4 cases are triggered. Do not push! Thanks for the rescue, Marcel, but I was so focused on non-en, that I forgot I was just trying to trigger the first case. Why complicate things, when simple works? DO NOT PUSH the proof patch. :) Why not leave this test: cmp_deeply( ( C4::Templates::themelanguage( C4::Context->config('intrahtdocs'), 'about.tt' , 'intranet', 'fake_cgi' ) )[2], [ 'prog' ], 'We only expect one prog here' ); The other tests do not touch the returned themes,, (In reply to Marcel de Rooy from comment #8) > Why not leave this test: > cmp_deeply( ( C4::Templates::themelanguage( > C4::Context->config('intrahtdocs'), 'about.tt' , 'intranet', 'fake_cgi' ) > )[2], [ 'prog' ], 'We only expect one prog here' ); > > The other tests do not touch the returned themes,, Oops... Good point. Created attachment 59821 [details] [review] Bug 18036: Replace accidentally deleted test. Created attachment 59822 [details] [review] Bug 18036: Full complement of testing Added theme, lang, and availablethemes tests for all 4 cases. Created attachment 59823 [details] [review] Bug 18036: All 4 cases tested Added theme, lang, and availablethemes tests for all 4 cases. Previously just availablethemes was tested. Created attachment 59824 [details] [review] Proof that all 4 cases are triggered. Created attachment 59825 [details] [review] Bug 18036: Followup If you have the proof applied, do not apply this. If you do not have the proof applied, apply this. Created attachment 59838 [details] [review] Bug 18036: All 4 cases tested Added theme, lang, and availablethemes tests for all 4 cases. Previously just availablethemes was tested. Created attachment 59839 [details] [review] [DO NOT PUSH] Proof that all 4 cases are triggered. Comment on attachment 59839 [details] [review] [DO NOT PUSH] Proof that all 4 cases are triggered. Worked fine. Obsoleting it now. Created attachment 59843 [details] [review] Bug 18036: Perlcritic improve t/db_dependent/Templates.t TEST PLAN --------- 1) apply 17982 2) perlcritic -3 t/db_dependent/Templates.t -- messages 3) apply this patch 4) perlcritic -3 t/db_dependent/Templates.t -- OK 5) run koha qa test tools Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 59844 [details] [review] Bug 18036: All 4 cases tested Added theme, lang, and availablethemes tests for all 4 cases. Previously just availablethemes was tested. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> The warns from Mark's additional patch showed all four cases. Created attachment 59845 [details] [review] Bug 18036: [QA Follow-up] Descriptions and mock_preference Adjusted a few test descriptions. The test depends on template being set to prog. Made that explicit by adding a mock_preference. If you change the mock to bootstrap, this test will fail. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 60351 [details] [review] Bug 18036: Perlcritic improve t/db_dependent/Templates.t TEST PLAN --------- 1) apply 17982 2) perlcritic -3 t/db_dependent/Templates.t -- messages 3) apply this patch 4) perlcritic -3 t/db_dependent/Templates.t -- OK 5) run koha qa test tools Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 60352 [details] [review] Bug 18036: All 4 cases tested Added theme, lang, and availablethemes tests for all 4 cases. Previously just availablethemes was tested. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> The warns from Mark's additional patch showed all four cases. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 60353 [details] [review] Bug 18036: [QA Follow-up] Descriptions and mock_preference Adjusted a few test descriptions. The test depends on template being set to prog. Made that explicit by adding a mock_preference. If you change the mock to bootstrap, this test will fail. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 17.05, thanks Mark, Marcel! Marked as enh, but more tests... I was tempted, but this doesn't apply cleanly on 16.11.x for some reason. This won't get ported back to 16.11.x as it is an enhancement. (In reply to Katrin Fischer from comment #25) > Marked as enh, but more tests... I was tempted, but this doesn't apply > cleanly on 16.11.x for some reason. Depends on 17892. I could rework for 16.11.x if you really want it. :) > This won't get ported back to 16.11.x as it is an enhancement. But you are right this is an enhancement ... of tests. :) I think it's ok to leave out, but thx Mark. |