Add check links test

[bollwyvl]

Thanks for nbsphinx!

This adds a minimal link checking step with https://github.com/minrk/pytest-check-links. Hooray tests!

Unfortunately, the reason I'm making this PR is I think there's a regression on 0.5.1 that generates broken links... but it's hard to provide a minimal reproducing test case (or explore a fix) when there's no tests!

Stay tuned for the follow on!

[bollwyvl]

I've generally found it's not nice to launch a mini DDoS on github and sphinx by checking external links, which leads to flake, which leads to more requests, so it just ignores everything on http.

Also, none of the ipynb links are rewritten (so they all break), even though pytest-check-links knows how to handle them.

[mgeier]

Also, none of the ipynb links are rewritten (so they all break)

What do you mean by that?
Who is supposed to rewrite which links?

BTW, the tests on this PR are passing, but your descriptions make me think they are supposed to fail, aren't they?

[bollwyvl]

since we're not really testing code, and pytest-check-links does create a fair number of warnings, just ignoring everything for now.

[mgeier]

Thanks for this PR!

The links (including http links) are already tested on CircleCI (Hooray tests!):

nbsphinx/.circleci/config.yml

Lines 79 to 82 in e988192

	- run:
	name: Checking for Outdated Links
	command: |
	python3 setup.py build_sphinx -W -b linkcheck

So your tests are checking the same links again, or am I missing something?

I think there's a regression on 0.5.1 that generates broken links... but it's hard to provide a minimal reproducing test case (or explore a fix) when there's no tests!

Well there are tests, and they are passing.
Granted, the Sphinx linkcheck builder checks the links without building the HTML files.
But if there is a bug in the rendering of HTML links, that's a Sphinx bug, which should be tested for in Sphinx (and I guess Sphinx has tests for this?).

How do those broken links look like?

[bollwyvl]

Well there are tests, and they are passing.

Great, sorry: a quick check of README, CONTRIBUTING.md, and git grep test came back negative. Didn't think to look in the other CI for check. Perhaps adding something in one of those places about the test regime?

without building the HTML files.

That sounds great, for unit testing. At the end of the day, though, if style, scripts, images, etc. get added by this plugin (or plugins it brings along or advocates) with broken links, or it fails to generate things that sphinx even recognizes are links (it's rather cavalier about what extension things need to have) then the product the user builds can be broken even though sphinx is working exactly as designed.

How do those broken links look like?

It's in fact the latter example where multiple formats come into play: on this pr i've got ipynb pointing to md files (that unfortunately get generated). Docs files did not change (yet) during the pull request, but going back to nbsphinx==0.5.0 let the generated content actually pass. A concrete example is here, where this link:

Returns a [named map of starters](../schema/v2.md#definitions-group-all-starters), as loaded via `traitlets`. 

is still coming up as md in the output on 0.5.1, but in one of the generated outputs (in this case rst). :

_ /home/vsts/work/1/s/docs/_build/doctrees/nbsphinx/notebooks/REST API.ipynb: ../schema/v2.md#definitions-group-all-starters _

So either there is new content being created in my build in 0.5.0 that is unwanted, or it's breaking something that I did want, for some reason. I confess, I'm not much of a sphinx hacker, and could really care less about rst... hence my use of nbsphinsx. This PR was initial attempt to get a small amount of integration testing added, which could be gradually expanded to include a repro of the case above of my experience 0.5.1, but better characterized and less complicated.

[mgeier]

Perhaps adding something in one of those places about the test regime?

I've created #399, does that help?

At the end of the day, though, if style, scripts, images, etc. get added by this plugin (or plugins it brings along or advocates) with broken links, or it fails to generate things that sphinx even recognizes are links (it's rather cavalier about what extension things need to have) then the product the user builds can be broken even though sphinx is working exactly as designed.

Yes, something like this could happen ...

However, I hope nbsphinx doesn't do any of those bad things you mentioned.
It shouldn't ever create HTML links on its own without going through Sphinx first.

i've got ipynb pointing to md files (that unfortunately get generated).

As long as the .md files are in the source directory while Sphinx is running, this shouldn't be a problem.

I've tried your example locally (with the master branch of nbsphinx), and it seems to work fine.

Sphinx is actually quite picky regarding local links, but it only generates warnings if something is wrong. You should use the -W flag to turn warnings into errors.

But in your example, there are no warnings for the link you mentioned. There is an unrelated warning, though:

docs/notebooks/Use Cases.ipynb:25: WARNING: undefined label: /notebooks/starters.ipynb#starter%20tree%20url (if the link has no caption the label must precede a section header)

This is a legitimate warning, you should replace %20 by -, then it should work.

[...] but in one of the generated outputs (in this case rst).

What do you mean by that?
Are you expecting .rst output somewhere?

Can you please elaborate what's the actual problem?

[mgeier]

@bollwyvl Can you please answer my questions? Is the problem still occurring?

[mgeier]

@bollwyvl Do you think this PR should replace the existing link testing on CircleCI?

I don't think we should test the links twice, right?

Unfortunately, the reason I'm making this PR is I think there's a regression on 0.5.1 that generates broken links... but it's hard to provide a minimal reproducing test case (or explore a fix) when there's no tests!

Can you please provide an example for those broken links?

Stay tuned for the follow on!

I'm still tuned!

[mgeier]

@bollwyvl Any news on this?

In the meantime, I've moved the linkcheck from CircleCI to Github Actions: https://github.com/spatialaudio/nbsphinx/blob/master/.github/workflows/linkcheck.yml, see #491.

There have also been some improvements in linkcheck itself: sphinx-doc/sphinx#7985

[bollwyvl]

I'm not really using nbsphinx anymore, can close this.