Bug 12653 - PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests
Summary: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Bernardo Gonzalez Kriegel
QA Contact: Galen Charlton
URL:
Keywords:
Depends on:
Blocks: 11346
  Show dependency treegraph
 
Reported: 2014-07-24 15:48 UTC by Owen Leonard
Modified: 2015-06-04 23:23 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 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests (8.50 KB, patch)
2014-08-11 06:29 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests (8.93 KB, patch)
2014-08-11 15:35 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests (20.75 KB, patch)
2014-10-17 11:43 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests (20.86 KB, patch)
2014-10-20 12:55 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests (20.86 KB, patch)
2014-10-20 12:58 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests (20.86 KB, patch)
2014-10-20 14:44 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 12653: Text file clean up (9.73 KB, patch)
2014-10-20 15:00 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED OFF] Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests (17.37 KB, patch)
2014-10-20 15:44 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 12653: Text file clean up (9.73 KB, patch)
2014-10-20 15:44 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12653: Text file clean up (9.88 KB, patch)
2014-10-21 18:40 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
k (17.36 KB, patch)
2014-10-25 19:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests (17.43 KB, patch)
2014-10-25 19:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12653: Text file clean up (9.98 KB, patch)
2014-10-25 19:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12653: (RM followup) Fix the test count (1006 bytes, patch)
2014-10-29 23:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 12653: remaining prog reference in t/db_dependent/SuggestionEngine_ExplodedTerms.t (1.18 KB, patch)
2014-10-30 02:39 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2014-07-24 15:48:17 UTC
The following tests contain references to opac-tmpl/prog:

  xt/single_quotes.t
  xt/author/valid-templates.t
  xt/author/icondirectories.t
  xt/author/translatable-templates.t
  xt/tt_valid.t
Comment 1 Bernardo Gonzalez Kriegel 2014-08-11 06:29:34 UTC Comment hidden (obsolete)
Comment 2 Bernardo Gonzalez Kriegel 2014-08-11 15:35:02 UTC Comment hidden (obsolete)
Comment 3 Bernardo Gonzalez Kriegel 2014-08-11 17:12:35 UTC
To test more:

1) Edit some TT file (opac/staff) and add somewhere something like

<span [% IF ( value ) %]name="name"[% ELSE %] js="_('">TEST [% INCLUDE 'fail.inc' %]</span>

Save and run the tests

  xt/single_quotes.t
  xt/author/valid-templates.t
  xt/tt_valid.t

must fail

2) To fail icondirectories.t the only way I found is ranaming/removing itemtypeimg dir,
$ mv koha-tmpl/opac-tmpl/bootstrap/itemtypeimg koha-tmpl/opac-tmpl/bootstrap/item

If the directory exists, even if empty, the test pass.
Before patch it pass even deleting the icon dir...

3) To fail translatable-templates.t, simply change the name of 'en' template dir for your current staff (prog) or opac (bootstrap/prog/ccsr)
Comment 4 Bernardo Gonzalez Kriegel 2014-08-11 23:16:06 UTC
Need to think this again
I was told that this tests should not depend on DB values.
Comment 5 Bernardo Gonzalez Kriegel 2014-10-17 11:43:15 UTC Comment hidden (obsolete)
Comment 6 Mark Tompsett 2014-10-20 04:32:29 UTC
Comment on attachment 32477 [details] [review]
Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests

Review of attachment 32477 [details] [review]:
-----------------------------------------------------------------

::: xt/author/translatable-templates.t
@@ +27,4 @@
>  
>  =cut
>  
> +use Test::More tests => 4; # FIXME: this must be set to total number of themes opac+staff

We should not be adding "FIXME" when modifying it to this should not be difficult. The number of themes (opac+staff) in known by line 49. Just add a calculation to set $number_of_tests_run.

  use Test::More;
  ... run your tests ...
  done_testing( $number_of_tests_run );
Comment 7 Mark Tompsett 2014-10-20 04:35:38 UTC
Comment on attachment 32477 [details] [review]
Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests

Review of attachment 32477 [details] [review]:
-----------------------------------------------------------------

::: xt/author/valid-templates.t
@@ +87,2 @@
>  
>  done_testing();

And this is how you do it, when you don't feel like counting.
Comment 8 Mark Tompsett 2014-10-20 12:26:43 UTC
Comment on attachment 32477 [details] [review]
Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests

Review of attachment 32477 [details] [review]:
-----------------------------------------------------------------

::: xt/single_quotes.t
@@ +43,4 @@
>  my @files;
>  find(
>      sub {
>          open my $fh, $_ or die "Could not open $_: $!";

Since @themes has no /en, I would have expected something here.

@@ +46,5 @@
>          open my $fh, $_ or die "Could not open $_: $!";
>          my @lines = sort grep /\_\(\'/, <$fh>;
>          push @files, { name => "$_", lines => \@lines } if @lines;
> +    },
> +    @themes

Note: @themes does not have /en on the values in the array.
Not that it should only be en, but this likely tests the wrong directory level.

@@ -31,5 @@
> -    },qw#
> -    ./koha-tmpl/opac-tmpl/prog/en
> -    ./koha-tmpl/opac-tmpl/bootstrap/en
> -    ./koha-tmpl/intranet-tmpl/prog/en
> -    #

Note: all these have /en on them.
Comment 9 Mark Tompsett 2014-10-20 12:43:58 UTC
(In reply to M. Tompsett from comment #8)
> Comment on attachment 32477 [details] [review] [review]
> Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in
> tests
> 
> Review of attachment 32477 [details] [review] [review]:
> -----------------------------------------------------------------

Well, my eyeball failed me. A simple print statement confirmed it is good.
Comment 10 Bernardo Gonzalez Kriegel 2014-10-20 12:55:28 UTC
Hi Mark, thanks for testing

About Comment #6 and #7, I'll add a 'done_testing()'
About Comment #8, it's one level up, I'll fix that
Comment 11 Bernardo Gonzalez Kriegel 2014-10-20 12:55:58 UTC Comment hidden (obsolete)
Comment 12 Bernardo Gonzalez Kriegel 2014-10-20 12:58:49 UTC Comment hidden (obsolete)
Comment 13 Mark Tompsett 2014-10-20 13:28:40 UTC
Comment on attachment 32529 [details] [review]
Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests

Review of attachment 32529 [details] [review]:
-----------------------------------------------------------------

::: xt/tt_valid.t
@@ +29,5 @@
> +# OPAC themes
> +my $opac_dir  = 'koha-tmpl/opac-tmpl';
> +opendir ( my $dh, $opac_dir ) or die "can't opendir $opac_dir: $!";
> +for my $theme ( grep { not /^\.|lib|js/ } readdir($dh) ) {
> +    push @themes, "$opac_dir/$theme";

/en is missing.

@@ +37,5 @@
> +# STAFF themes
> +my $staff_dir = 'koha-tmpl/intranet-tmpl';
> +opendir ( $dh, $staff_dir ) or die "can't opendir $staff_dir: $!";
> +for my $theme ( grep { not /^\.|lib|js/ } readdir($dh) ) {
> +    push @themes, "$staff_dir/$theme";

/en is missing.

@@ +59,4 @@
>          }
>          ($dir) = $dir =~ /koha-tmpl\/(.*)$/;
>          push @files, { name => "$dir/$name", lines => \@lines } if @lines;
> +      }, @themes

However, since this is a find, it still works without the /en.
Comment 14 Mark Tompsett 2014-10-20 14:06:09 UTC
./opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_COPYING.txt
./opac-tmpl/ccsr/itemtypeimg/crystal-clear/_COPYING.txt
-- these lack two empty lines at the end of the file

./intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt
./opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_COPYING.txt
-- which these two files have

Can someone else confirm their files differ this way?
Comment 15 Mark Tompsett 2014-10-20 14:20:12 UTC
(In reply to M. Tompsett from comment #14)
> ./opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_COPYING.txt
> ./opac-tmpl/ccsr/itemtypeimg/crystal-clear/_COPYING.txt
> -- these lack two empty lines at the end of the file
> 
> ./intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt
> ./opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_COPYING.txt
> -- which these two files have
> 
> Can someone else confirm their files differ this way?

Also had issues with _README.txt, but not the same differences.
Comment 16 Bernardo Gonzalez Kriegel 2014-10-20 14:35:14 UTC
(In reply to M. Tompsett from comment #14)
> ./opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_COPYING.txt
> ./opac-tmpl/ccsr/itemtypeimg/crystal-clear/_COPYING.txt
> -- these lack two empty lines at the end of the file
> 
> ./intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt
> ./opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_COPYING.txt
> -- which these two files have
> 
> Can someone else confirm their files differ this way?


On current master (not my patch), 
~/kohaclone/koha-tmpl$ diff ./intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt ./opac-tmpl/ccsr/itemtypeimg/crystal-clear/_COPYING.txt
121,122d120
< 
< 

CCSR is not fixed, differs in more than that file. But because is about to be removed soon, I skipped the test there. There is no point to update/fix itemtypeimg for ccsr (perhaps yes for <= 3.16 versions)

About your next comment, I fix only files needed to make test pass.

I'll resubmit because I forgot to add a later fix (a missing '/en' on tt_valid.t, it checks more than necessary)
Comment 17 Bernardo Gonzalez Kriegel 2014-10-20 14:44:27 UTC Comment hidden (obsolete)
Comment 18 Mark Tompsett 2014-10-20 15:00:01 UTC Comment hidden (obsolete)
Comment 19 Mark Tompsett 2014-10-20 15:00:51 UTC
Remove the file clean up in your patch, and I'll sign off.
Then you sign off on my Text file clean up. :)
Comment 20 Mark Tompsett 2014-10-20 15:36:10 UTC
Comment on attachment 32540 [details] [review]
Bug 12653: Text file clean up

Wasn't needed. :)
Comment 21 Mark Tompsett 2014-10-20 15:37:19 UTC
Comment on attachment 32540 [details] [review]
Bug 12653: Text file clean up

Oops. Poor testing. Is needed.
Comment 22 Mark Tompsett 2014-10-20 15:44:28 UTC Comment hidden (obsolete)
Comment 23 Mark Tompsett 2014-10-20 15:44:36 UTC Comment hidden (obsolete)
Comment 24 Mark Tompsett 2014-10-20 16:15:27 UTC
I ran the test plan as given in comment #22, but I needed to have my patch applied to get it to pass. Bernardo Gonzalez Kriegel, please test my patch, sign off, and change the status of this bug to signed off. :)
Comment 25 Bernardo Gonzalez Kriegel 2014-10-21 18:40:53 UTC Comment hidden (obsolete)
Comment 26 Katrin Fischer 2014-10-25 19:44:46 UTC Comment hidden (obsolete)
Comment 27 Katrin Fischer 2014-10-25 19:45:54 UTC
Created attachment 32744 [details] [review]
[PASSED QA] Bug 12653: PROG/CCSR deprecation: Correct hard-coded opac-tmpl/prog path in tests

This patch removes explicit references of themes from

  xt/author/icondirectories.t
  xt/author/translatable-templates.t
  xt/author/valid-templates.t
  xt/single_quotes.t
  xt/tt_valid.t

For xt/author/icondirectories.t it fixes a small difference on
two bootstrap files, previously unchecked, crystal-clear/_COPYING.txt
and crystal-clear/_README.txt

Some updates to license information

To test:
1. Apply the patch
2. Run each test, all must pass
prove xt/author/icondirectories.t xt/author/translatable-templates.t xt/author/valid-templates.t xt/single_quotes.t xt/tt_valid.t

3. Try to fail each test

a) xt/author/icondirectories.t
create a new file on any of icon dirs, e.g.
touch koha-tmpl/opac-tmpl/bootstrap/itemtypeimg/newfile.png
test must fail

b) xt/author/translatable-templates.t
change the name of 'en' dir on any theme opac or staff
test must fail

Add following line to any template file
<span [% IF ( value ) %]name="name"[% ELSE %] js="_('">TEST [% INCLUDE 'fail.inc' %]</span>

c) xt/author/valid-templates.t must fail
d) xt/single_quotes.t must fail
e) xt/tt_valid.t must fail

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Katrin Fischer 2014-10-25 19:46:09 UTC
Created attachment 32745 [details] [review]
[PASSED QA] Bug 12653: Text file clean up

_README.txt has 4 newlines removed.
_COPYING.txt has 2 empty lines at the end of the file removed.
This was done by copying the smallest file in an existing theme
directory.

TEST PLAN
---------
1) Compare files.
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt koha-tmpl/opac-tmpl/prog/itemtypeimg/crystal-clear/_COPYING.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt koha-tmpl/opac-tmpl/ccsr/itemtypeimg/crystal-clear/_COPYING.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt koha-tmpl/opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_COPYING.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_README.txt koha-tmpl/opac-tmpl/prog/itemtypeimg/crystal-clear/_README.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_README.txt koha-tmpl/opac-tmpl/ccsr/itemtypeimg/crystal-clear/_README.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_README.txt koha-tmpl/opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_README.txt
-- there will be differences.
2) Apply patch
3) Compare files again.
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt koha-tmpl/opac-tmpl/prog/itemtypeimg/crystal-clear/_COPYING.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt koha-tmpl/opac-tmpl/ccsr/itemtypeimg/crystal-clear/_COPYING.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_COPYING.txt koha-tmpl/opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_COPYING.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_README.txt koha-tmpl/opac-tmpl/prog/itemtypeimg/crystal-clear/_README.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_README.txt koha-tmpl/opac-tmpl/ccsr/itemtypeimg/crystal-clear/_README.txt
diff koha-tmpl/intranet-tmpl/prog/img/itemtypeimg/crystal-clear/_README.txt koha-tmpl/opac-tmpl/bootstrap/itemtypeimg/crystal-clear/_README.txt
-- there will be no differences.
4) git reset --hard origin/master
5) Apply the patch affecting the .t files.
6) Prove things.
prove xt/author/icondirectories.t xt/author/translatable-templates.t xt/author/valid-templates.t xt/single_quotes.t xt/tt_valid.t
-- expecting xt/author/icondirectories.t to fail
7) Apply this patch.
8) Prove things.
prove xt/author/icondirectories.t xt/author/translatable-templates.t xt/author/valid-templates.t xt/single_quotes.t xt/tt_valid.t
-- expecting success.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

This patch fixes some issues with files on itemtypes dirs.
No koha-qa erros.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 29 Chris Cormack 2014-10-26 02:21:47 UTC
Pushed to MM-OPAC/theme_dep
Comment 30 Tomás Cohen Arazi 2014-10-29 23:45:53 UTC
Created attachment 32953 [details] [review]
Bug 12653: (RM followup) Fix the test count

Bug 12233 removed the prog and CCSR theme directories, so the number
of tests needs to be fixed.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 31 Tomás Cohen Arazi 2014-10-30 00:55:12 UTC
Patches merged into master.

Thansk Bernardo and Mark!
Comment 32 Tomás Cohen Arazi 2014-10-30 02:39:16 UTC
Created attachment 32959 [details] [review]
Bug 12653: remaining prog reference in t/db_dependent/SuggestionEngine_ExplodedTerms.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>