Bug 18036 - Improve test coverage for themelanguage
Summary: Improve test coverage for themelanguage
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Mark Tompsett
QA Contact:
URL:
Keywords:
Depends on: 17982
Blocks:
  Show dependency treegraph
 
Reported: 2017-02-02 07:54 UTC by Marcel de Rooy
Modified: 2017-12-07 22:15 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 18036: Improve test coverage for themelanguage (3.78 KB, patch)
2017-02-02 08:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18036: Perlcritic improve t/db_dependent/Templates.t (1.51 KB, patch)
2017-02-02 17:19 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18036: All 4 cases tested (2.40 KB, patch)
2017-02-02 17:20 UTC, Mark Tompsett
Details | Diff | Splinter Review
Proof that all 4 cases are triggered. (1.18 KB, patch)
2017-02-02 17:20 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18036: Replace accidentally deleted test. (1.26 KB, patch)
2017-02-02 19:05 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18036: Full complement of testing (2.57 KB, patch)
2017-02-02 19:16 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18036: All 4 cases tested (3.09 KB, patch)
2017-02-02 19:19 UTC, Mark Tompsett
Details | Diff | Splinter Review
Proof that all 4 cases are triggered. (1.18 KB, patch)
2017-02-02 19:19 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18036: Followup (1.08 KB, patch)
2017-02-02 20:05 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18036: All 4 cases tested (3.07 KB, patch)
2017-02-03 01:02 UTC, Mark Tompsett
Details | Diff | Splinter Review
[DO NOT PUSH] Proof that all 4 cases are triggered. (1.21 KB, patch)
2017-02-03 01:02 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18036: Perlcritic improve t/db_dependent/Templates.t (1.61 KB, patch)
2017-02-03 07:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18036: All 4 cases tested (3.23 KB, patch)
2017-02-03 07:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18036: [QA Follow-up] Descriptions and mock_preference (3.37 KB, patch)
2017-02-03 07:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18036: Perlcritic improve t/db_dependent/Templates.t (1.64 KB, patch)
2017-02-16 14:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18036: All 4 cases tested (3.26 KB, patch)
2017-02-16 14:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18036: [QA Follow-up] Descriptions and mock_preference (3.40 KB, patch)
2017-02-16 14:12 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2017-02-02 07:54:27 UTC
See bug 17982
Comment 1 Marcel de Rooy 2017-02-02 08:18:46 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
Comment 2 Marcel de Rooy 2017-02-02 08:19:32 UTC
Saved a kitten for you, Mark. But still needs some polishing..
Comment 3 Mark Tompsett 2017-02-02 13:15:21 UTC
(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.
Comment 4 Mark Tompsett 2017-02-02 17:19:58 UTC
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
Comment 5 Mark Tompsett 2017-02-02 17:20:10 UTC
Created attachment 59819 [details] [review]
Bug 18036: All 4 cases tested
Comment 6 Mark Tompsett 2017-02-02 17:20:28 UTC
Created attachment 59820 [details] [review]
Proof that all 4 cases are triggered.

Do not push!
Comment 7 Mark Tompsett 2017-02-02 17:23:05 UTC
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. :)
Comment 8 Marcel de Rooy 2017-02-02 18:35:20 UTC
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,,
Comment 9 Mark Tompsett 2017-02-02 18:58:17 UTC
(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.
Comment 10 Mark Tompsett 2017-02-02 19:05:38 UTC
Created attachment 59821 [details] [review]
Bug 18036: Replace accidentally deleted test.
Comment 11 Mark Tompsett 2017-02-02 19:16:29 UTC
Created attachment 59822 [details] [review]
Bug 18036: Full complement of testing

Added theme, lang, and availablethemes tests for all 4 cases.
Comment 12 Mark Tompsett 2017-02-02 19:19:48 UTC
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.
Comment 13 Mark Tompsett 2017-02-02 19:19:58 UTC
Created attachment 59824 [details] [review]
Proof that all 4 cases are triggered.
Comment 14 Mark Tompsett 2017-02-02 20:05:34 UTC
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.
Comment 15 Mark Tompsett 2017-02-03 01:02:14 UTC
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.
Comment 16 Mark Tompsett 2017-02-03 01:02:22 UTC
Created attachment 59839 [details] [review]
[DO NOT PUSH] Proof that all 4 cases are triggered.
Comment 17 Marcel de Rooy 2017-02-03 07:11:18 UTC
Comment on attachment 59839 [details] [review]
[DO NOT PUSH] Proof that all 4 cases are triggered.

Worked fine. Obsoleting it now.
Comment 18 Marcel de Rooy 2017-02-03 07:12:23 UTC
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>
Comment 19 Marcel de Rooy 2017-02-03 07:12:27 UTC
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.
Comment 20 Marcel de Rooy 2017-02-03 07:12:31 UTC
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>
Comment 21 Jonathan Druart 2017-02-16 14:12:22 UTC
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>
Comment 22 Jonathan Druart 2017-02-16 14:12:30 UTC
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>
Comment 23 Jonathan Druart 2017-02-16 14:12:33 UTC
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>
Comment 24 Kyle M Hall 2017-02-17 15:46:13 UTC
Pushed to master for 17.05, thanks Mark, Marcel!
Comment 25 Katrin Fischer 2017-02-19 20:43:31 UTC
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.
Comment 26 Mark Tompsett 2017-02-20 01:11:07 UTC
(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. :)
Comment 27 Katrin Fischer 2017-02-20 06:24:04 UTC
I think it's ok to leave out, but thx Mark.