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

Added link checking to tox and release.py #5614

Merged
merged 2 commits into from
Nov 21, 2019

Conversation

steffenschroeder
Copy link
Contributor

@steffenschroeder steffenschroeder commented Jul 16, 2019

Fixes #1722 by checking all links in documentation during the release process.
Should only be merged after all broken links are fixed (#5613)

It's running during release in release.py or can be triggered manually by executing tox -e docs-checklinks.

False positives can be added to list linkcheck_ignore in doc/en/conf.py.

@nicoddemus
Copy link
Member

nicoddemus commented Aug 1, 2019

Hi,

First of all thanks @steffenschroeder for the work on this! We appreciate it.

I've just tried this branch and there are still link check errors. It takes a while, but the worst part is that it doesn't really show a summary of the broken links, forcing me to scroll and hunt for the missing ones on the terminal. Am I missing something?

Executing tox -e docs-checklinks | grep broken, I get:

(line   44) broken    http://pytest.org/2.0.0/assert.html#newreport - Anchor 'newreport' not found
(line   10) broken    http://pytest.org/2.0.0/index.html - 404 Client Error: Not Found for url: http://pytest.org/2.0.0/index.html
(line   39) broken    http://pytest.org/2.0.0/usage.html - 404 Client Error: Not Found for url: http://pytest.org/2.0.0/usage.html
(line   53) broken    http://pytest.org/2.0.0/customize.html - 404 Client Error: Not Found for url: http://pytest.org/2.0.0/customize.html
(line   59) broken    http://pytest.org/latest/plugins.html#cmdunregister - Anchor 'cmdunregister' not found
(line   77) broken    http://pytest.org/latest/mark.html - 404 Client Error: Not Found for url: http://pytest.org/latest/mark.html
(line   21) broken    http://pytest.org/latest/example/markers.html - 404 Client Error: Not Found for url: http://pytest.org/latest/example/markers.html
(line   12) broken    http://pytest.org/latest/example/parametrize.html - 404 Client Error: Not Found for url: http://pytest.org/latest/example/parametrize.html
(line   30) broken    http://pytest.org/latest/funcarg_compare.html - 404 Client Error: Not Found for url: http://pytest.org/latest/funcarg_compare.html
(line   36) broken    http://pytest.org/latest/getting-started.html - 404 Client Error: Not Found for url: http://pytest.org/latest/getting-started.html
(line   21) broken    http://pytest.org/latest/unittest.html - 404 Client Error: Not Found for url: http://pytest.org/latest/unittest.html
(line   16) broken    http://pytest.org/latest/fixture.html - 404 Client Error: Not Found for url: http://pytest.org/latest/fixture.html
(line   97) broken    http://pytest.org/latest/faq.html - 404 Client Error: Not Found for url: http://pytest.org/latest/faq.html
(line   16) broken    http://pytest.org/latest/example/parametrize.html - 404 Client Error: Not Found for url: http://pytest.org/latest/example/parametrize.html
(line    9) broken    http://pytest.org/latest/yieldfixture.html - 404 Client Error: Not Found for url: http://pytest.org/latest/yieldfixture.html
(line   54) broken    https://pytest.org/latest/contributing.html - 404 Client Error: Not Found for url: https://pytest.org/latest/contributing.html
(line  126) broken    https://pytest.org/latest/usage.html#modifying-python-traceback-printing - Anchor 'modifying-python-traceback-printing' not found
(line 3124) broken    https://docs.pytest.org/en/latest/builtin.html#pytest.approx - Anchor 'pytest.approx' not found
(line 4476) broken    https://pytest.org/latest/usage.html#modifying-python-traceback-printing - Anchor 'modifying-python-traceback-printing' not found
(line 5000) broken    https://pytest.org/latest/contributing.html - 404 Client Error: Not Found for url: https://pytest.org/latest/contributing.html
(line 5739) broken    http://pytest.org/latest/example/parametrize.html - 404 Client Error: Not Found for url: http://pytest.org/latest/example/parametrize.html
(line 5875) broken    http://pytest.org/latest/faq.html - 404 Client Error: Not Found for url: http://pytest.org/latest/faq.html
(line 5984) broken    http://pytest.org/latest/mark.html - 404 Client Error: Not Found for url: http://pytest.org/latest/mark.html
(line 6170) broken    http://pytest.org/plugins.html#cmdunregister - Anchor 'cmdunregister' not found
(line   31) broken    http://code.activestate.com/pypm/ - HTTPConnectionPool(host='code.activestate.com', port=80): Max retries exceeded with url: /pypm/ (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x000001B02609D908>: Failed to establish a new connection: [WinError 10060] Uma tentativa de conex▒o falhou porque o componente conectado n▒o respondeu\r\ncorretamente ap▒s um per▒odo de tempo ou a conex▒o estabelecida falhou\r\nporque o host conectado n▒o respondeu',))

A lot of those seem to me to come from old CHANGELOG entries or the CHANGELOG itself, which I'm not sure how valuable it is in practice to fix links on the entry for really old releases such as 2.0.0.

An idea: would it be possible to only check links in the actual documentation? Those definitely should be up-to-date.

@steffenschroeder
Copy link
Contributor Author

The verbose output (showing working links) can be suppressed by adding ‘-q’ to the call of the sphinx-build. I had that initially but removed per per previous suggestion.
Can add it again.
The builder has also an option to disable the check of html anchors.
Unfortunately it has no option to exclude files (like old change log).
So could come up with some own link checker, enhance the sphinx linkchecker.
Alternatively we could continue checking everything.

@The-Compiler
Copy link
Member

FWIW I'd prefer fixing the links everywhere (or removing them if they aren't relevant anymore)

@nicoddemus
Copy link
Member

The verbose output (showing working links) can be suppressed by adding ‘-q’ to the call of the sphinx-build. I had that initially but removed per per previous suggestion.
Can add it again.

Indeed: #5614 (comment). Personally I prefer -q because then we only see the failing links, instead of a ton of text which then forces you to "hunt" for the broken links. @blueyed?

Also we need volunteers to fix the broken links before we can merge this.

@steffenschroeder
Copy link
Contributor Author

I already fixed a couple of broken links with another PR.
If there are still some broken ones left, I can fix them (once I‘m back from vacation in 2 weeks).

@steffenschroeder steffenschroeder force-pushed the checklinks branch 2 times, most recently from 03f97d3 to 9b440a0 Compare August 27, 2019 19:04
@steffenschroeder
Copy link
Contributor Author

Removed the verbose output (only broken links are shown) and fixed the remaining broken links.

@steffenschroeder steffenschroeder force-pushed the checklinks branch 2 times, most recently from adcc602 to 75d7234 Compare September 15, 2019 16:19
@steffenschroeder
Copy link
Contributor Author

Hi,
this has unfortunately lost some traction.(I acknowledge that this is not high priority though)

I fixed again all broken links and updated this PR.

For me, I see the following options to continue:

  1. This PR get merged as it is. (Link checking mandatory during release process)
  2. We make the check for the broken optional (maybe through a command line flat to release.py)
  3. We decouple link checking from release process (leave it in tox but remove it from release.py) to let maintainers or volunteers do the checking from time to time.
  4. We remove the link checking completely and only keep the already fixed broken links.
  5. We close the PR without further action.
  6. something completely different :-)

All of the above is fine for me and if there is some follow up, I'm happy to work and that.
But I think we should decide on the next step and close this one.

@steffenschroeder steffenschroeder force-pushed the checklinks branch 2 times, most recently from cf3ebd4 to 5477cbe Compare September 21, 2019 20:45
@blueyed
Copy link
Contributor

blueyed commented Nov 5, 2019

Can you rebase it to have green CI at least for now?
(might have done it but learnt that some folks do not like it)

@nicoddemus
Copy link
Member

@steffenschroeder thanks again for your patience on this! 👍

I've tested the branch just now and tox -e docs-checklinks took ~4mins and completed successfully, which I believe it is acceptable time when cutting a release, and broken links should be far and between.

I propose we merge this (after rebasing as suggested by @blueyed) and see how it fits on the release process, as it is not something we can't rethink later.

@steffenschroeder
Copy link
Contributor Author

Thanks for considering this to merge.
I rebased the branch.

@pytest-dev pytest-dev deleted a comment from codecov bot Nov 7, 2019
@Zac-HD
Copy link
Member

Zac-HD commented Nov 21, 2019

Close-and-reopening to restart CI...

@Zac-HD Zac-HD closed this Nov 21, 2019
@Zac-HD Zac-HD reopened this Nov 21, 2019
@Zac-HD Zac-HD merged commit 7e10c81 into pytest-dev:master Nov 21, 2019
@Zac-HD
Copy link
Member

Zac-HD commented Nov 21, 2019

Thanks again @steffenschroeder! This is a really useful change which will help the many people who read our docs, and I really admire your persistence in seeing it through ❤️

@blueyed
Copy link
Contributor

blueyed commented Nov 21, 2019

Thanks indeed!

Just for reference, since I've tried it:

31.80s user 1.70s system 17% cpu 3:14.54 total

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.

do linkchecks for changelog lint
5 participants