Skip to content

Commit 8b33ee2

Browse files
nik9000nik9000
authored
Teach Kibana link checking about PR features (#898)
The docs build has three features when building all books: 1. If you specify `--keep_hash` we reuse the hash of the source repos for every book that we've built before. This is useful for rebuilding only the books that we converted to asciidoctor. Or in combination with `--sub_dir` 2. If you specify `--sub_dir` we use a directory instead of a commit hash from some source repo. This is mostly useful in combination with `--keep_hash` because when you use those together your can rebuild the docs as though you committed the directory that you are subsituting. It makes for perfect pull request tests. 3. We automatically check all of the links, including links from some typescript files in the kibana repo. The trouble is that the kibana repo link checking process didn't understand `--keep_hash` and `--sub_dir`. This commit changes that. The bulk of it the change is testing the link checking, which we didn't really have any of up until now. Closes #892
1 parent f884092 commit 8b33ee2

File tree

7 files changed

+320
-36
lines changed

7 files changed

+320
-36
lines changed

‎build_docs.pl

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -311,21 +311,34 @@ sub check_kibana_links {
311311
my @branches = sort map { $_->basename }
312312
grep { $_->is_dir } $build_dir->subdir('en/kibana')->children;
313313

314+
my $link_check_name = 'link-check-kibana';
315+
314316
for (@branches) {
315317
$branch = $_;
316318
next if $branch eq 'current' || $branch =~ /^\d/ && $branch lt 5;
317319
say " Branch $branch";
320+
my $links_file;
318321
my $source = eval {
319-
$repo->show_file( $branch, $src_path . ".js" )
322+
$links_file = $src_path . ".js";
323+
$repo->show_file( $link_check_name, $branch, $links_file );
324+
} || eval {
325+
$links_file = $src_path . ".ts";
326+
$repo->show_file( $link_check_name, $branch, $links_file );
320327
} || eval {
321-
$repo->show_file( $branch, $src_path . ".ts" )
328+
$links_file = $legacy_path . ".js";
329+
$repo->show_file( $link_check_name, $branch, $links_file );
322330
} || eval {
323-
$repo->show_file( $branch, $legacy_path . ".js" )
324-
} ||
325-
$repo->show_file( $branch, $legacy_path . ".ts" );
331+
$links_file = $legacy_path . ".ts";
332+
$repo->show_file( $link_check_name, $branch, $links_file );
333+
};
334+
die "failed to find kibana links file;\n$@" unless $source;
326335

327336
$link_checker->check_source( $source, $extractor,
328-
"Kibana [$branch]: $src_path" );
337+
"Kibana [$branch]: $links_file" );
338+
339+
# Mark the file that we need for the link check done so we can use
340+
# --keep_hash with it during some other build.
341+
$repo->mark_done( $link_check_name, $branch, $links_file, 0 );
329342
}
330343
}
331344

‎integtest/spec/all_books_broken_link_detection_spec.rb

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
# frozen_string_literal: true
2+
3+
##
4+
# Assertions about when books are rebuilt based on changes in source
5+
# repositories or the book's configuration.
6+
RSpec.describe 'building all books' do
7+
KIBANA_LINKS_FILE = 'src/ui/public/documentation_links/documentation_links.js'
8+
shared_context 'there is a broken link in the docs' do |check_links|
9+
convert_before do |src, dest|
10+
repo = src.repo_with_index 'repo', 'https://www.elastic.co/guide/foo'
11+
book = src.book 'Test'
12+
book.source repo, 'index.asciidoc'
13+
convert = dest.prepare_convert_all src.conf
14+
convert.skip_link_check unless check_links
15+
convert.convert(expect_failure: check_links)
16+
end
17+
end
18+
shared_context 'there is a broken link in kibana' do |check_links|
19+
convert_before do |src, dest|
20+
# Kibana is special and we check links in it with a little magic
21+
kibana_repo = src.repo 'kibana'
22+
kibana_repo.write KIBANA_LINKS_FILE, <<~JS
23+
export const documentationLinks = {
24+
foo: `${ELASTIC_WEBSITE_URL}guide/foo`,
25+
};
26+
JS
27+
kibana_repo.commit 'init'
28+
29+
# The preview of the book is important here because it is how we detect
30+
# the versions of kibana to check.
31+
# NOCOMMIT maybe we shouldn't?!
32+
repo = src.repo_with_index 'repo', "Doesn't matter"
33+
book = src.book 'Test', prefix: 'en/kibana'
34+
book.source repo, 'index.asciidoc'
35+
convert = dest.prepare_convert_all src.conf
36+
convert.skip_link_check unless check_links
37+
convert.convert(expect_failure: check_links)
38+
end
39+
end
40+
41+
describe 'when broken link detection is disabled' do
42+
describe 'when there is a broken link in the docs' do
43+
include_context 'there is a broken link in the docs', false
44+
it 'logs that it skipped link checking' do
45+
expect(outputs[0]).to include('Skipped Checking links')
46+
end
47+
end
48+
describe 'when there is a broken link in kibana' do
49+
include_context 'there is a broken link in kibana', false
50+
it 'logs that it skipped link checking' do
51+
expect(outputs[0]).to include('Skipped Checking links')
52+
end
53+
end
54+
end
55+
describe 'when broken link detection is enabled' do
56+
shared_examples 'all links are ok' do
57+
it 'logs that all the links are ok' do
58+
expect(outputs[-1]).to include('All cross-document links OK')
59+
end
60+
end
61+
shared_examples 'there are broken links in kibana' do
62+
it 'logs there are bad cross document links' do
63+
expect(outputs[-1]).to include('Bad cross-document links:')
64+
end
65+
it 'logs the bad link' do
66+
expect(outputs[-1]).to include(indent(<<~LOG.strip, ' '))
67+
Kibana [master]: src/ui/public/documentation_links/documentation_links.js:
68+
- foo
69+
LOG
70+
end
71+
end
72+
describe 'when all of the links are intact' do
73+
convert_before do |src, dest|
74+
repo = src.repo_with_index(
75+
'repo',
76+
'https://www.elastic.co/guide/test/current/chapter.html'
77+
)
78+
book = src.book 'Test'
79+
book.source repo, 'index.asciidoc'
80+
dest.prepare_convert_all(src.conf).convert
81+
end
82+
include_examples 'all links are ok'
83+
end
84+
describe 'when there is a broken link in the docs' do
85+
include_context 'there is a broken link in the docs', true
86+
it 'logs there are bad cross document links' do
87+
expect(outputs[0]).to include('Bad cross-document links:')
88+
end
89+
it 'logs the bad link' do
90+
expect(outputs[0]).to include(indent(<<~LOG.strip, ' '))
91+
/tmp/docsbuild/target_repo/html/test/current/chapter.html:
92+
- foo
93+
LOG
94+
end
95+
end
96+
describe 'when there is a broken link in kibana' do
97+
include_context 'there is a broken link in kibana', true
98+
include_examples 'there are broken links in kibana'
99+
end
100+
describe 'when using --keep_hash and --sub_dir together like a PR test' do
101+
describe 'when there is a broken link in one of the books being built' do
102+
convert_before do |src, dest|
103+
repo1 = src.repo_with_index 'repo1', "Doesn't matter"
104+
book1 = src.book 'Test1'
105+
book1.source repo1, 'index.asciidoc'
106+
repo2 = src.repo_with_index 'repo2', "Also doesn't matter"
107+
book2 = src.book 'Test2'
108+
book2.source repo2, 'index.asciidoc'
109+
dest.prepare_convert_all(src.conf).convert
110+
111+
repo2.write 'index.asciidoc', <<~ASCIIDOC
112+
= Title
113+
114+
[[chapter]]
115+
== Chapter
116+
https://www.elastic.co/guide/foo
117+
ASCIIDOC
118+
dest.prepare_convert_all(src.conf)
119+
.keep_hash
120+
.sub_dir(repo2, 'master')
121+
.convert(expect_failure: true)
122+
end
123+
it 'logs there are bad cross document links' do
124+
expect(outputs[1]).to include('Bad cross-document links:')
125+
end
126+
it 'logs the bad link' do
127+
expect(outputs[1]).to include(indent(<<~LOG.strip, ' '))
128+
/tmp/docsbuild/target_repo/html/test2/current/chapter.html:
129+
- foo
130+
LOG
131+
end
132+
end
133+
describe "when there is a broken link in a book that isn't being built" do
134+
convert_before do |src, dest|
135+
repo1 = src.repo_with_index 'repo1', "Doesn't matter"
136+
book1 = src.book 'Test1'
137+
book1.source repo1, 'index.asciidoc'
138+
repo2 = src.repo_with_index 'repo2', "Also doesn't matter"
139+
book2 = src.book 'Test2'
140+
book2.source repo2, 'index.asciidoc'
141+
dest.prepare_convert_all(src.conf).convert
142+
143+
repo1.write 'index.asciidoc', <<~ASCIIDOC
144+
= Title
145+
146+
[[chapter]]
147+
== Chapter
148+
https://www.elastic.co/guide/foo
149+
ASCIIDOC
150+
dest.prepare_convert_all(src.conf)
151+
.keep_hash
152+
.sub_dir(repo2, 'master')
153+
.convert
154+
end
155+
include_examples 'all links are ok'
156+
end
157+
describe 'when there is a broken link in kibana' do
158+
def self.setup(src, dest)
159+
kibana_repo = src.repo_with_index 'kibana', "Doesn't matter"
160+
kibana_repo.write KIBANA_LINKS_FILE, 'no links here'
161+
kibana_repo.commit 'add empty links file'
162+
kibana_book = src.book 'Kibana', prefix: 'en/kibana'
163+
kibana_book.source kibana_repo, 'index.asciidoc'
164+
repo2 = src.repo_with_index 'repo2', "Also doesn't matter"
165+
book2 = src.book 'Test2'
166+
book2.source repo2, 'index.asciidoc'
167+
dest.prepare_convert_all(src.conf).convert
168+
169+
kibana_repo.write KIBANA_LINKS_FILE, <<~JS
170+
export const documentationLinks = {
171+
foo: `${ELASTIC_WEBSITE_URL}guide/foo`,
172+
};
173+
JS
174+
end
175+
describe 'when the broken link is in an unbuilt branch' do
176+
convert_before do |src, dest|
177+
setup src, dest
178+
src.repo('kibana').commit 'add bad link'
179+
dest.prepare_convert_all(src.conf)
180+
.keep_hash
181+
.sub_dir(src.repo('repo2'), 'master')
182+
.convert
183+
end
184+
include_examples 'all links are ok'
185+
end
186+
describe 'when the broken link is in an *new* unbuilt branch' do
187+
convert_before do |src, dest|
188+
setup src, dest
189+
kibana = src.repo('kibana')
190+
kibana.switch_to_new_branch 'new_branch'
191+
kibana.commit 'add bad link'
192+
dest.prepare_convert_all(src.conf)
193+
.keep_hash
194+
.sub_dir(src.repo('repo2'), 'master')
195+
.convert
196+
end
197+
include_examples 'all links are ok'
198+
end
199+
describe 'when the broken link is in the --sub_dir' do
200+
convert_before do |src, dest|
201+
setup src, dest
202+
dest.prepare_convert_all(src.conf)
203+
.keep_hash
204+
.sub_dir(src.repo('kibana'), 'master')
205+
.convert(expect_failure: true)
206+
end
207+
include_examples 'there are broken links in kibana'
208+
end
209+
end
210+
end
211+
end
212+
end

‎integtest/spec/helper/book.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def source(repo, path, map_branches: nil)
3636
def conf
3737
# We can't use to_yaml here because it emits yaml 1.2 but the docs build
3838
# only supports 1.0.....
39-
<<~YAML.split("\n").map { |s| ' ' + s }.join "\n"
39+
indent(<<~YAML, ' ')
4040
title: #{@title}
4141
prefix: #{@prefix}
4242
current: master
@@ -69,7 +69,7 @@ def sources_conf
6969
YAML
7070
yaml += map_branches_conf config[:map_branches]
7171
end
72-
yaml.split("\n").map { |s| ' ' + s }.join "\n"
72+
indent(yaml, ' ')
7373
end
7474

7575
def map_branches_conf(map_branches)

‎integtest/spec/helper/dest.rb

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,53 @@ def convert_single(from, to,
4747
run_convert(cmd, expect_failure)
4848
end
4949

50+
def prepare_convert_all(conf)
51+
ConvertAll.new conf, bare_repo, self
52+
end
53+
5054
##
5155
# Convert a conf file worth of books and check it out.
5256
def convert_all(conf, expect_failure: false, target_branch: nil)
53-
cmd = %W[
54-
--all
55-
--push
56-
--target_repo #{bare_repo}
57-
--conf #{conf}
58-
]
59-
cmd += ['--target_branch', target_branch] if target_branch
60-
run_convert(cmd, expect_failure)
57+
# TODO: remove this in favor of prepare_convert_all
58+
convert = ConvertAll.new conf, bare_repo, self
59+
convert.target_branch target_branch if target_branch
60+
convert.convert(expect_failure: expect_failure)
61+
end
62+
63+
class ConvertAll
64+
def initialize(conf, target_repo, dest)
65+
@cmd = %W[
66+
--all
67+
--push
68+
--target_repo #{target_repo}
69+
--conf #{conf}
70+
]
71+
@dest = dest
72+
end
73+
74+
def convert(expect_failure: false)
75+
@dest.run_convert(@cmd, expect_failure)
76+
end
77+
78+
def target_branch(target_branch)
79+
@cmd += ['--target_branch', target_branch]
80+
self
81+
end
82+
83+
def skip_link_check
84+
@cmd += ['--skiplinkcheck']
85+
self
86+
end
87+
88+
def keep_hash
89+
@cmd += ['--keep_hash']
90+
self
91+
end
92+
93+
def sub_dir(repo, branch)
94+
@cmd += ['--sub_dir', "#{repo.name}:#{branch}:#{repo.root}"]
95+
self
96+
end
6197
end
6298

6399
##
@@ -68,8 +104,6 @@ def checkout_conversion(branch: nil)
68104
sh "git clone #{branch_cmd}#{bare_repo} #{@dest}"
69105
end
70106

71-
private
72-
73107
##
74108
# The location of the bare repository. the first time this is called in a
75109
# given context the bare repository is initialized

‎integtest/spec/spec_helper.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ def files_in(dir)
3737
end
3838
end
3939

40+
def indent(str, indentation)
41+
str.split("\n").map { |s| indentation + s }.join "\n"
42+
end
43+
4044
##
4145
# Match paths that refer to an existing file.
4246
# Prefer this instead of `expect(File).to exist('path')` because the failure

‎lib/ES/BaseRepo.pm

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,6 @@ sub _reference_args {
112112
return ();
113113
}
114114

115-
#===================================
116-
sub show_file {
117-
#===================================
118-
my $self = shift;
119-
my ( $branch, $file ) = @_;
120-
121-
local $ENV{GIT_DIR} = $self->git_dir;
122-
123-
return decode_utf8 run( qw (git show ), $branch . ':' . $file );
124-
}
125-
126115
#===================================
127116
sub name { shift->{name} }
128117
sub git_dir { shift->{git_dir} }

0 commit comments

Comments
 (0)