Teach Kibana link checking about PR features

build_docs.pl

@@ -311,21 +311,34 @@ sub check_kibana_links {
     my @branches = sort map { $_->basename }
         grep { $_->is_dir } $build_dir->subdir('en/kibana')->children;
 
+    my $link_check_name = 'link-check-kibana';
+
     for (@branches) {
         $branch = $_;
         next if $branch eq 'current' || $branch =~ /^\d/ && $branch lt 5;
         say "  Branch $branch";
+        my $links_file;
         my $source = eval {
-            $repo->show_file( $branch, $src_path . ".js" )
+            $links_file = $src_path . ".js";
+            $repo->show_file( $link_check_name, $branch, $links_file );
+        } || eval {
+            $links_file = $src_path . ".ts";
+            $repo->show_file( $link_check_name, $branch, $links_file );
         } || eval {
-            $repo->show_file( $branch, $src_path . ".ts" )
+            $links_file = $legacy_path . ".js";
+            $repo->show_file( $link_check_name, $branch, $links_file );
         } || eval {
-            $repo->show_file( $branch, $legacy_path . ".js" )
-        } ||
-            $repo->show_file( $branch, $legacy_path . ".ts" );
+            $links_file = $legacy_path . ".ts";
+            $repo->show_file( $link_check_name, $branch, $links_file );
+        };
+        die "failed to find kibana links file;\n$@" unless $source;
 
         $link_checker->check_source( $source, $extractor,
-            "Kibana [$branch]: $src_path" );
+            "Kibana [$branch]: $links_file" );
+
+        # Mark the file that we need for the link check done so we can use
+        # --keep_hash with it during some other build.
+        $repo->mark_done( $link_check_name, $branch, $links_file, 0 );
     }
 }
 

integtest/spec/all_books_broken_link_detection_spec.rb

@@ -0,0 +1,212 @@
+# frozen_string_literal: true
+
+##
+# Assertions about when books are rebuilt based on changes in source
+# repositories or the book's configuration.
+RSpec.describe 'building all books' do
+  KIBANA_LINKS_FILE = 'src/ui/public/documentation_links/documentation_links.js'
+  shared_context 'there is a broken link in the docs' do |check_links|
+    convert_before do |src, dest|
+      repo = src.repo_with_index 'repo', 'https://www.elastic.co/guide/foo'
+      book = src.book 'Test'
+      book.source repo, 'index.asciidoc'
+      convert = dest.prepare_convert_all src.conf
+      convert.skip_link_check unless check_links
+      convert.convert(expect_failure: check_links)
+    end
+  end
+  shared_context 'there is a broken link in kibana' do |check_links|
+    convert_before do |src, dest|
+      # Kibana is special and we check links in it with a little magic
+      kibana_repo = src.repo 'kibana'
+      kibana_repo.write KIBANA_LINKS_FILE, <<~JS
+        export const documentationLinks = {
+          foo: `${ELASTIC_WEBSITE_URL}guide/foo`,
+        };
+      JS
+      kibana_repo.commit 'init'
+
+      # The preview of the book is important here because it is how we detect
+      # the versions of kibana to check.
+      # NOCOMMIT maybe we shouldn't?!
+      repo = src.repo_with_index 'repo', "Doesn't matter"
+      book = src.book 'Test', prefix: 'en/kibana'
+      book.source repo, 'index.asciidoc'
+      convert = dest.prepare_convert_all src.conf
+      convert.skip_link_check unless check_links
+      convert.convert(expect_failure: check_links)
+    end
+  end
+
+  describe 'when broken link detection is disabled' do
+    describe 'when there is a broken link in the docs' do
+      include_context 'there is a broken link in the docs', false
+      it 'logs that it skipped link checking' do
+        expect(outputs[0]).to include('Skipped Checking links')
+      end
+    end
+    describe 'when there is a broken link in kibana' do
+      include_context 'there is a broken link in kibana', false
+      it 'logs that it skipped link checking' do
+        expect(outputs[0]).to include('Skipped Checking links')
+      end
+    end
+  end
+  describe 'when broken link detection is enabled' do
+    shared_examples 'all links are ok' do
+      it 'logs that all the links are ok' do
+        expect(outputs[-1]).to include('All cross-document links OK')
+      end
+    end
+    shared_examples 'there are broken links in kibana' do
+      it 'logs there are bad cross document links' do
+        expect(outputs[-1]).to include('Bad cross-document links:')
+      end
+      it 'logs the bad link' do
+        expect(outputs[-1]).to include(indent(<<~LOG.strip, '  '))
+          Kibana [master]: src/ui/public/documentation_links/documentation_links.js:
+           - foo
+        LOG
+      end
+    end
+    describe 'when all of the links are intact' do
+      convert_before do |src, dest|
+        repo = src.repo_with_index(
+          'repo',
+          'https://www.elastic.co/guide/test/current/chapter.html'
+        )
+        book = src.book 'Test'
+        book.source repo, 'index.asciidoc'
+        dest.prepare_convert_all(src.conf).convert
+      end
+      include_examples 'all links are ok'
+    end
+    describe 'when there is a broken link in the docs' do
+      include_context 'there is a broken link in the docs', true
+      it 'logs there are bad cross document links' do
+        expect(outputs[0]).to include('Bad cross-document links:')
+      end
+      it 'logs the bad link' do
+        expect(outputs[0]).to include(indent(<<~LOG.strip, '  '))
+          /tmp/docsbuild/target_repo/html/test/current/chapter.html:
+           - foo
+        LOG
+      end
+    end
+    describe 'when there is a broken link in kibana' do
+      include_context 'there is a broken link in kibana', true
+      include_examples 'there are broken links in kibana'
+    end
+    describe 'when using --keep_hash and --sub_dir together like a PR test' do
+      describe 'when there is a broken link in one of the books being built' do
+        convert_before do |src, dest|
+          repo1 = src.repo_with_index 'repo1', "Doesn't matter"
+          book1 = src.book 'Test1'
+          book1.source repo1, 'index.asciidoc'
+          repo2 = src.repo_with_index 'repo2', "Also doesn't matter"
+          book2 = src.book 'Test2'
+          book2.source repo2, 'index.asciidoc'
+          dest.prepare_convert_all(src.conf).convert
+
+          repo2.write 'index.asciidoc', <<~ASCIIDOC
+            = Title
+
+            [[chapter]]
+            == Chapter
+            https://www.elastic.co/guide/foo
+          ASCIIDOC
+          dest.prepare_convert_all(src.conf)
+              .keep_hash
+              .sub_dir(repo2, 'master')
+              .convert(expect_failure: true)
+        end
+        it 'logs there are bad cross document links' do
+          expect(outputs[1]).to include('Bad cross-document links:')
+        end
+        it 'logs the bad link' do
+          expect(outputs[1]).to include(indent(<<~LOG.strip, '  '))
+            /tmp/docsbuild/target_repo/html/test2/current/chapter.html:
+             - foo
+          LOG
+        end
+      end
+      describe "when there is a broken link in a book that isn't being built" do
+        convert_before do |src, dest|
+          repo1 = src.repo_with_index 'repo1', "Doesn't matter"
+          book1 = src.book 'Test1'
+          book1.source repo1, 'index.asciidoc'
+          repo2 = src.repo_with_index 'repo2', "Also doesn't matter"
+          book2 = src.book 'Test2'
+          book2.source repo2, 'index.asciidoc'
+          dest.prepare_convert_all(src.conf).convert
+
+          repo1.write 'index.asciidoc', <<~ASCIIDOC
+            = Title
+
+            [[chapter]]
+            == Chapter
+            https://www.elastic.co/guide/foo
+          ASCIIDOC
+          dest.prepare_convert_all(src.conf)
+              .keep_hash
+              .sub_dir(repo2, 'master')
+              .convert
+        end
+        include_examples 'all links are ok'
+      end
+      describe 'when there is a broken link in kibana' do
+        def self.setup(src, dest)
+          kibana_repo = src.repo_with_index 'kibana', "Doesn't matter"
+          kibana_repo.write KIBANA_LINKS_FILE, 'no links here'
+          kibana_repo.commit 'add empty links file'
+          kibana_book = src.book 'Kibana', prefix: 'en/kibana'
+          kibana_book.source kibana_repo, 'index.asciidoc'
+          repo2 = src.repo_with_index 'repo2', "Also doesn't matter"
+          book2 = src.book 'Test2'
+          book2.source repo2, 'index.asciidoc'
+          dest.prepare_convert_all(src.conf).convert
+
+          kibana_repo.write KIBANA_LINKS_FILE, <<~JS
+            export const documentationLinks = {
+              foo: `${ELASTIC_WEBSITE_URL}guide/foo`,
+            };
+          JS
+        end
+        describe 'when the broken link is in an unbuilt branch' do
+          convert_before do |src, dest|
+            setup src, dest
+            src.repo('kibana').commit 'add bad link'
+            dest.prepare_convert_all(src.conf)
+                .keep_hash
+                .sub_dir(src.repo('repo2'), 'master')
+                .convert
+          end
+          include_examples 'all links are ok'
+        end
+        describe 'when the broken link is in an *new* unbuilt branch' do
+          convert_before do |src, dest|
+            setup src, dest
+            kibana = src.repo('kibana')
+            kibana.switch_to_new_branch 'new_branch'
+            kibana.commit 'add bad link'
+            dest.prepare_convert_all(src.conf)
+                .keep_hash
+                .sub_dir(src.repo('repo2'), 'master')
+                .convert
+          end
+          include_examples 'all links are ok'
+        end
+        describe 'when the broken link is in the --sub_dir' do
+          convert_before do |src, dest|
+            setup src, dest
+            dest.prepare_convert_all(src.conf)
+                .keep_hash
+                .sub_dir(src.repo('kibana'), 'master')
+                .convert(expect_failure: true)
+          end
+          include_examples 'there are broken links in kibana'
+        end
+      end
+    end
+  end
+end

integtest/spec/helper/book.rb

@@ -36,7 +36,7 @@ def source(repo, path, map_branches: nil)
   def conf
     # We can't use to_yaml here because it emits yaml 1.2 but the docs build
     # only supports 1.0.....
-    <<~YAML.split("\n").map { |s| '    ' + s }.join "\n"
+    indent(<<~YAML, '    ')
       title:      #{@title}
       prefix:     #{@prefix}
       current:    master
@@ -69,7 +69,7 @@ def sources_conf
       YAML
       yaml += map_branches_conf config[:map_branches]
     end
-    yaml.split("\n").map { |s| '  ' + s }.join "\n"
+    indent(yaml, '  ')
   end
 
   def map_branches_conf(map_branches)

integtest/spec/helper/dest.rb

@@ -47,17 +47,53 @@ def convert_single(from, to,
     run_convert(cmd, expect_failure)
   end
 
+  def prepare_convert_all(conf)
+    ConvertAll.new conf, bare_repo, self
+  end
+
   ##
   # Convert a conf file worth of books and check it out.
   def convert_all(conf, expect_failure: false, target_branch: nil)
-    cmd = %W[
-      --all
-      --push
-      --target_repo #{bare_repo}
-      --conf #{conf}
-    ]
-    cmd += ['--target_branch', target_branch] if target_branch
-    run_convert(cmd, expect_failure)
+    # TODO: remove this in favor of prepare_convert_all
+    convert = ConvertAll.new conf, bare_repo, self
+    convert.target_branch target_branch if target_branch
+    convert.convert(expect_failure: expect_failure)
+  end
+
+  class ConvertAll
+    def initialize(conf, target_repo, dest)
+      @cmd = %W[
+        --all
+        --push
+        --target_repo #{target_repo}
+        --conf #{conf}
+      ]
+      @dest = dest
+    end
+
+    def convert(expect_failure: false)
+      @dest.run_convert(@cmd, expect_failure)
+    end
+
+    def target_branch(target_branch)
+      @cmd += ['--target_branch', target_branch]
+      self
+    end
+
+    def skip_link_check
+      @cmd += ['--skiplinkcheck']
+      self
+    end
+
+    def keep_hash
+      @cmd += ['--keep_hash']
+      self
+    end
+
+    def sub_dir(repo, branch)
+      @cmd += ['--sub_dir', "#{repo.name}:#{branch}:#{repo.root}"]
+      self
+    end
   end
 
   ##
@@ -68,8 +104,6 @@ def checkout_conversion(branch: nil)
     sh "git clone #{branch_cmd}#{bare_repo} #{@dest}"
   end
 
-  private
-
   ##
   # The location of the bare repository. the first time this is called in a
   # given context the bare repository is initialized

integtest/spec/spec_helper.rb

@@ -37,6 +37,10 @@ def files_in(dir)
   end
 end
 
+def indent(str, indentation)
+  str.split("\n").map { |s| indentation + s }.join "\n"
+end
+
 ##
 # Match paths that refer to an existing file.
 # Prefer this instead of `expect(File).to exist('path')` because the failure

lib/ES/BaseRepo.pm

@@ -112,17 +112,6 @@ sub _reference_args {
     return ();
 }
 
-#===================================
-sub show_file {
-#===================================
-    my $self = shift;
-    my ( $branch, $file ) = @_;
-
-    local $ENV{GIT_DIR} = $self->git_dir;
-
-    return decode_utf8 run( qw (git show ), $branch . ':' . $file );
-}
-
 #===================================
 sub name          { shift->{name} }
 sub git_dir       { shift->{git_dir} }