Test documentation links against a TOC file

[ShimShtein]

Instead of testing against the production documentation, test against a TOC file that can be generated from non-published documentation.

[ekohl]

Suggested change

	all_links.each do |key, doc_address|
	next unless doc_address.match?(/html/)
	failed[key] = doc_address unless checker.test_link(doc_address)
	end
	failed = all_links.filter { |_key, doc_address| !doc_address.include?('html') && checker.test_link(doc_address) }

[ShimShtein]

@ekohl Done, thanks!

[evgeni]

Where and how will be obtain (and host) the TOC file for the upcoming release and then execute this as part of the regular CI?
My concern is that with todays setup we have "test links for N+1 release against prod N docs" which works in 80% of the cases, while with the current state of this PR we effectively move to "test links on demand by some magic process" (as the TOC file is not in the PR) thus possibly missing things when a PR does change the link but CI doesn't turn red.

Also, what about GA'ed versions, should we there also test against a TOC, even tho we do have published docs to test against?

[ShimShtein]

Where and how will be obtain (and host) the TOC file for the upcoming release and then execute this as part of the regular CI?

In my vision this will become a github action that we will be able to trigger on changes to either docs repo or theme repo.
The task should look like this:

1. Make sure we have ruby and asciidoctor are installed
2. Clone the theme repo
3. Clone the documentation repo
4. In the documentation repo run make task that generates the TOC file
5. In the theme repo, run the rake task

The make task that generates TOC for upstream docs lives here: theforeman/foreman-documentation#2665. There is also a similar downstream PR that makes sure the downstream TOC is generated.

I would vote against storing the TOC file somewhere since it's cheap to generate, although we can add it as a build artifact.

Even though we can test against the GA'd docs, I suggest to reuse the same procedure. It is a matter of which git tag to check out on the docs repo clone, so I don't think it's a big difference.

One of the advantages of this procedure, is the ability to test anchors validity (we didn't have it with the old tests by default).

[evgeni]

How do we get GitHub to access the downstream docs repo?

[ShimShtein]

Unfortunately for downstream, we will have to do it from the downstream side, unless there will be a decision to give access to the TOC file at least from upstream.

This task accepts a filename_to_link file that we can beef up and use for translating upstream names in the TOC to downstream names (or translate downstream links to their upstream counterparts). This would give us the ability to test downstream links against the upstream documentation. This will not give us full confidence for topics specific to downstream, but at least for common ones - it will do the trick.

[evgeni]

This smells like a source for regressions then (= when a PR to this repo can't verify the links automatically).

[ShimShtein]

Added TOC file to the fixtures directory. The file would be updated by automatic PRs.

[evgeni]

the same link exists in DOCS_GUIDES_LINKS but wasn't updated?

[evgeni]

ForemanThemeSatellite::Documentation::DOCS_GUIDES_LINKS is untested

[ShimShtein]

Added DOCS_GUIDES_LINKS hash too

[ShimShtein]

I don't think the test failures are related. Ready for another round

[evgeni]

Suggested change

	%r{/html/(.*)}.match(path)&.[](1)
	%r{/html/(.*)}.match(path).to_a[1]

that's the same, right? I was starring at that &.[] too long.

(nil.to_a is [], so it works also when there is no match)

[evgeni]

Suggested change

	split = path.match(%r{([^/]*)/(.*)})
	first = split&.[](1) || path
	rest = split&.[](2)
	first, rest = path.split('/', 2)

[evgeni]

% cat /tmp/bench.rb 
require 'benchmark/ips'

path = "something/very/long/but/we/dont/care"

Benchmark.ips do |x|
  x.report("regex") do
    split = path.match(%r{([^/]*)/(.*)})
    first = split&.[](1) || path
    rest = split&.[](2)
  end

  x.report("split") do
    first, rest = path.split('/', 2)
  end
end

% ruby /tmp/bench.rb   
Warming up --------------------------------------
               regex   145.613k i/100ms
               split   315.618k i/100ms
Calculating -------------------------------------
               regex      1.423M (± 1.3%) i/s -      7.135M in   5.014002s
               split      3.064M (± 0.7%) i/s -     15.465M in   5.047714s

;-)

[evgeni]

the test_docs_redirect_branded failure is related, as you changed the link here.

[evgeni]

see inline comments.

ack once resolved