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: Main
Hardware: All All
: P5 - low normal
Assignee: Bernardo Gonzalez Kriegel
QA Contact: Galen Charlton
URL:
Keywords:
Depends on:
Blocks: 11346
  Show dependency tree / graph
 
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:
Circulation function:

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 (tcohen) 		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 (tcohen) 		Details | Diff | Splinter Review
Show Obsolete (11) View All Add an attachment (proposed patch, testcase, etc.)

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 (tcohen) 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 (tcohen) 2014-10-30 00:55:12 UTC 
Patches merged into master.

Thansk Bernardo and Mark!
Comment 32 Tomás Cohen Arazi (tcohen) 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>