Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add linkcheck test for the docs #6543

Merged
merged 8 commits into from
Feb 17, 2020

Conversation

s-weigand
Copy link
Contributor

Since it is hard for big documentations to keep all links up with changes in a project, it's a good idea to check the links are working, to prevent broken links as in #6525 and #6541.

Luckily sphinx has this feature built in, so I added it to the test suite.

To prevent the test suite from false positive breaking, i.e. if a website is temporarily down, I made the test allow_failures in the travis build matrix.

I also added some url patterns to be ignored by the test, since they are only an example case, are local development only or test false positive since the ref is generated by javascript.

@stsewd
Copy link
Member

stsewd commented Jan 20, 2020

I'm -1 on putting this on travis, but +1 on having this in our tox env. The reason is that linkcheck is really slow. And we should be using the ref role for our internal links (the docs fail when there is a broken ref). Also, the problem from #6541 is for another reason.

@s-weigand
Copy link
Contributor Author

True that linkcheck takes very long, but the codecov test takes even longer, so it wouldn't add an additional delay for the whole test suite to finish.

@stsewd
Copy link
Member

stsewd commented Jan 20, 2020

True that linkcheck takes very long, but the codecov test takes even longer, so it wouldn't add an additional delay for the whole test suite to finish.

Fair enough. Tests take ~20 min, and linkcheck takes ~11 min

tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
Copy link
Member

@stsewd stsewd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@stsewd stsewd requested a review from a team January 20, 2020 18:19
docs/conf.py Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
docs/conf.py Show resolved Hide resolved
@mgeier
Copy link
Contributor

mgeier commented Jan 21, 2020

You can probably speed it up if you add a few more ignores:

linkcheck_ignore = [
    ...,
    r'https://github\.com/readthedocs/readthedocs\.org/issues',
    r'https://github\.com/readthedocs/readthedocs\.org/pull',
    r'https://docs\.readthedocs\.io/\?rtd_search',
    r'https://readthedocs\.org/search',
]

@s-weigand
Copy link
Contributor Author

@mgeier Confirmed the speedup locally, 3min instead of 10min.

@s-weigand s-weigand force-pushed the add-linkcheck-tests branch from be30dd8 to 9b2b724 Compare January 22, 2020 15:57
the links refer to the issues and PRs of readthedocs.org on github, as well as the search pages of readthedocs.io and docs.readthedocs.io
@s-weigand s-weigand force-pushed the add-linkcheck-tests branch from 9b2b724 to d70c062 Compare January 22, 2020 16:11
@stsewd stsewd requested a review from a team January 22, 2020 16:38
Copy link
Contributor

@davidfischer davidfischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine. While I feel our tests already take forever, this seems like a worthwhile check.

There are definitely some broken links in our docs (mostly example code) that could be fixed.

@stsewd stsewd merged commit 59611eb into readthedocs:master Feb 17, 2020
@s-weigand s-weigand deleted the add-linkcheck-tests branch February 17, 2020 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants