Only 2 of the pages in the app test now generate invalid links.

Also memoize the link checking to make the app test much faster
fix various warnings and bad uris
Fix the path_to function to convert every / to %2F.

diff --git a/lib/Gitalist/View/Default.pm b/lib/Gitalist/View/Default.pm
index 443704b..0c8d576 100644 (file)
--- a/lib/Gitalist/View/Default.pm
+++ b/lib/Gitalist/View/Default.pm
@@ -18,7 +18,9 @@ use Template::Stash;
 
 # define list method to flatten arrayrefs
 $Template::Stash::LIST_OPS->{ to_path } = sub {
-    return join('%2F', shift->flatten, @_);
+    my $path = join('%2F', shift->flatten, @_);
+    $path =~ s{/}{%2F}g;
+    return $path;
 };
 
 __PACKAGE__->meta->make_immutable(inline_constructor => 0);

diff --git a/root/_diff.tt2 b/root/_diff.tt2
index 8577bf6..34618bf 100644 (file)
--- a/root/_diff.tt2
+++ b/root/_diff.tt2
@@ -1,9 +1,9 @@
 [% INCLUDE inc/syntax_highlight_css.tt2 %]
 [% FOREACH item IN diff %]
 <div class='diff-head'>
- diff --git
- <a href='[% c.uri_for_action("/commit/blob", [Repository.name, item.src], item.file) %]'>[% item.a %]</a>
- <a href='[% c.uri_for_action("/commit/blob", [Repository.name, item.dst], item.file) %]'>[% item.b %]</a>
+ diff --git [%# FIXME %]
+ <a href='[% c.uri_for_action("/commit/blob", [Repository.name, Commit.sha1], item.file) %]'>[% item.a %][% item %]</a>
+ <a href='[% c.uri_for_action("/commit/blob", [Repository.name, Commit.sha1], item.file) %]'>[% item.b %]</a>
 </div>
 <div class='diff-index'>
  [% item.index %]

diff --git a/root/fragment/commit/blame.tt2 b/root/fragment/commit/blame.tt2
index 75f139e..d5db5f6 100644 (file)
--- a/root/fragment/commit/blame.tt2
+++ b/root/fragment/commit/blame.tt2
@@ -7,13 +7,13 @@
        styleinfo = " style='border-top: solid 3px #" _ linecolour _ ";'"; -%]
    <td nowrap class='author'[% styleinfo %]>[% info.commit.author %]</td>
    <td nowrap class='date'[% styleinfo %]>[% info.commit.author_dt.ymd %]</td>
-   <td nowrap class='commit-info chroma-hash'[% styleinfo %]><a title="[% info.commit.author %] on [% info.commit.author_dt %]" href='[% c.uri_for('commit', {h=info.commit.sha1}) %]'>[% linecolour %]</td>
+   <td nowrap class='commit-info chroma-hash'[% styleinfo %]><a title="[% info.commit.author %] on [% info.commit.author_dt %]" href='[% c.uri_for_action('/commit/commit', [Repository.name, info.commit.sha1]) %]'>[% linecolour %]</td>
      [%- ELSE -%]
      <td nowrap class='author'></td>
      <td nowrap class='date'></td>
      <td nowrap class='commit-info'></td>
      [%- END -%]
-   <td nowrap class='lineno' id='l[% info.meta.lineno %]' style='border-right: solid 3px #[% linecolour %]; border-left: solid 3px #[% linecolour %];'><tt><a href='[% c.uri_for('blame', {h=info.commit.sha1,f=filename}) %]#l[% info.meta.orig_lineno %]'>[% info.meta.lineno %]</a></tt></td>
+   <td nowrap class='lineno' id='l[% info.meta.lineno %]' style='border-right: solid 3px #[% linecolour %]; border-left: solid 3px #[% linecolour %];'><tt><a href='[% c.uri_for_action('/commit/blame', [Repository.name, info.commit.sha1], filename.to_path ) %]#l[% info.meta.orig_lineno %]'>[% info.meta.lineno %]</a></tt></td>
    <td nowrap class='data'><pre>[% blame_lines.${loop.index} %]</pre></td>
    [% lastsha1 = info.commit.sha1 %]
  </tr>

diff --git a/root/fragment/commit/diff_fancy.tt2 b/root/fragment/commit/diff_fancy.tt2
index 3a6ad5e..9554934 100644 (file)
--- a/root/fragment/commit/diff_fancy.tt2
+++ b/root/fragment/commit/diff_fancy.tt2
@@ -1,4 +1,4 @@
-MOO[%
+[%
   # In the case of merge commits there will be no diff tree.
   IF (diff_tree.size||0) > 0;
     INCLUDE '_diff_tree.tt2';
@@ -9,7 +9,7 @@ MOO[%
   %]
   <div class='no-difference'>
   [%
-    IF commit.parents > 1;
+    IF commit && commit.parents > 1;
       'Trivial merge';
     ELSE;
       'No differences found';

diff --git a/root/fragment/repository/heads.tt2 b/root/fragment/repository/heads.tt2
index bb7a359..d5ea26a 100644 (file)
--- a/root/fragment/repository/heads.tt2
+++ b/root/fragment/repository/heads.tt2
@@ -20,10 +20,10 @@
    <td class='sha1' title='[% head.sha1 %]'>[% INCLUDE 'inc/chroma_hash.tt2' sha1 = head.sha1.substr(0,7) %]</td>
    <td class='time-since' title='[% head.last_change %]'>[% time_since(head.last_change) %]</td>
    <td class='head[% head.sha1 == HEAD ? ' current' : '' %]'>[% head.name %]</td>
-   <td class='action-list'>
-     <a href="[% c.uri_for("shortlog", {h='refs/heads/' _ head.name}) %]">shortlog</a>
-     <a href="[% c.uri_for("log", {h='refs/heads/' _ head.name}) %]">log</a>
-     <a href="[% c.uri_for("tree", {h='refs/heads/' _ head.name, hb=head.name}) %]">tree</a>
+   <td class='action-list'>[%# FIXME %]
+     <a href="[% c.uri_for_action('/repository/shortlog', [Repository.name]) %]">shortlog</a>
+     <a href="[% c.uri_for_action('/repository/log', [Repository.name]) %]">log</a>
+     <a href="[% c.uri_for_action('/commit/tree', [Repository.name, head.name]) %]">tree</a>
    </td>
   </tr>
  [% END %]

diff --git a/root/nav/path.tt2 b/root/nav/path.tt2
index eef339c..c588cf0 100644 (file)
--- a/root/nav/path.tt2
+++ b/root/nav/path.tt2
@@ -1,7 +1,7 @@
 <div class='path'>
- <a href="[% c.uri_for("tree", {hb=head.sha1}) %]">[% Repository.name %]</a>
+ <a href="[% c.uri_for_action("/commit/tree", [Repository.name, head.sha1]) %]">[% Repository.name %]</a>
  [% FOREACH part IN filename.split('/') %]
   [% path = loop.first ? part : path _ '/' _ part %]
-  / <a href="[% c.uri_for(loop.last ? 'blob' : 'tree', {hb=head.sha1,f=path}) %]">[% part %]</a>
+  / <a href="[% c.uri_for_action(loop.last ? '/commit/blob' : '/commit/tree', [Repository.name, head.sha1], path.to_path) %]">[% part %]</a>
  [% END %]
 </div>

diff --git a/t/lib/TestGitalist.pm b/t/lib/TestGitalist.pm
index 05422de..0dce3b4 100644 (file)
--- a/t/lib/TestGitalist.pm
+++ b/t/lib/TestGitalist.pm
@@ -19,17 +19,38 @@ BEGIN {
     constant->import('MECH', $mech );
 }
 
+# Rechecking the same link multiple times is slow and lame!
+# Nicked this from WWW::Mechanize and memoized it...
+my %seen_links;
+sub Test::WWW::Mechanize::Catalyst::page_links_ok {
+    my $self = shift;
+    my $desc = shift;
+
+    $desc = 'All links ok' unless defined $desc;
+
+    my @links = $self->followable_links();
+    my @urls = Test::WWW::Mechanize::_format_links(\@links);
+
+    my @failures = $self->_check_links_status( [ grep { ! $seen_links{$_}++ } @urls ] );
+    my $ok = (@failures==0);
+
+    ok( $ok, $desc );
+    diag( $_ ) for @failures;
+
+    return $ok;
+}
+
+
 sub test_uri {
     my ($uri, $qs) = @_;
-    $qs ||= '';
     my $request = "/$uri"; 
     $request .= "?$qs" if defined $qs;
     my $response = request($request);
-    ok($response->is_success, "ok $uri - $qs");
+    ok($response->is_success, "ok $request");
     if (MECH) {
         my $res = MECH()->get($request);
-        ok $res->is_success, "ok mech $uri - $qs (" . $res->code . ')';
-        MECH()->page_links_ok()
+        ok $res->is_success, "ok mech $request (" . $res->code . ')';
+        MECH()->page_links_ok("All links ok from $request")
             if $res->content_type =~ m|text/html|;
     }
     return $response;