-
Notifications
You must be signed in to change notification settings - Fork 1
add doc:links and docs:links:check #427
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
base: main
Are you sure you want to change the base?
Conversation
|
@@ -24,6 +24,7 @@ At this time, PTB currently does not support setting up SonarQube for a **privat | |||
|
|||
## ✨ Features | |||
* #451: Added nox task to execute pysonar & added Sonar to the CI | |||
* #409: Doc link & checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* #409: Doc link & checks | |
* #409: Added nox tasks for `doc:links` and `docs:links:check` |
@@ -24,6 +24,7 @@ At this time, PTB currently does not support setting up SonarQube for a **privat | |||
|
|||
## ✨ Features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- At the top of the
docs/changes/unreleased.md
, please add to the ## Summary section. Below is some suggested text. - Please modify the suggested to explain why you selected certain values & what they do.
- Please, above the Sonar part, add
### Sonar
.
Links in Documentation
This version of the PTB adds nox tasks to check links present in our documentation:
docs:link
- List all the links within the documentationdocs:links:check
- Checks whether all links in the documentation are accessible
docs:links:check
is run in the CIchecks.yml
. If this step fails in the CI, please check the output & manually resolve the issues. There might be some cases where you need to update yourdoc/conf.py
with specific values for the allowed options for the Linkcheck Builder.We recommend the following values be added
linkcheck_rate_limit_timeout = 40 linkcheck_timeout = 5 linkcheck_delay = 20 linkcheck_retries = 2 linkcheck_anchors = False linkcheck_ignore: list[str] = [] linkcheck_allowed_redirects = { # All HTTP redirections from the source URI to # the canonical URI will be treated as "working". r"https://github\.com/.*": r"https://github\.com/login*" }
@@ -4,7 +4,7 @@ sphinx | |||
sphinx-multiversion | |||
+++++++++++++++++++ | |||
|
|||
The `sphinx-multiversion` extension is a modified copy of `Holzhaus/sphinx-multiversion <https://github.com/Holzhaus/sphinx-multiversion>`_. This copy was taken from version :code:`0.24.0`. | |||
The `sphinx-multiversion` extension is a modified copy of `Holzhaus/sphinx-multiversion <https://github.com/sphinx-contrib/multiversion>`_. This copy was taken from version :code:`0.24.0`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The repository has moved; thus, the reference text should be updated to reflect that.
The `sphinx-multiversion` extension is a modified copy of `Holzhaus/sphinx-multiversion <https://github.com/sphinx-contrib/multiversion>`_. This copy was taken from version :code:`0.24.0`. | |
The `sphinx-multiversion` extension is a modified copy of `sphinx-contrib/multiversion <https://github.com/sphinx-contrib/multiversion>`_. This copy was taken from version :code:`0.24.0`. |
@@ -78,3 +78,15 @@ | |||
"github_url": "https://github.com/exasol/python-toolbox", | |||
"accent_color": "grass", | |||
} | |||
# -- Configure link checking behavior ---------------------------------------- | |||
linkcheck_rate_limit_timeout = 40 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update project-template/{{cookiecutter.repo_name}}/doc/conf.py
with values that you think are worthwhile to have.
def docs_links_check(session: Session) -> None: | ||
"""Checks whether all links in the documentation are accessible.""" | ||
parser = argparse.ArgumentParser( | ||
prog="nox -s release:prepare", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prog="nox -s release:prepare", | |
prog="nox -s docs:links:check", |
"""Checks whether all links in the documentation are accessible.""" | ||
parser = argparse.ArgumentParser( | ||
prog="nox -s release:prepare", | ||
usage="nox -s release:prepare -- [-h] [-o |--output]", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
usage="nox -s release:prepare -- [-h] [-o |--output]", | |
usage="nox -s docs:link:check -- [-h] [-o |--output]", |
|
||
import nox | ||
import requests # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused import
import requests # type: ignore |
Container, | ||
Iterable, | ||
) | ||
from itertools import repeat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused import
from itertools import repeat |
import webbrowser | ||
from collections.abc import ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Container & Iterable are unused imports. Please remove.
from collections.abc import ( |
import argparse | ||
import json | ||
import os | ||
import re |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused import
import re |
@@ -1,11 +1,27 @@ | |||
from __future__ import annotations | |||
|
|||
import argparse | |||
import json | |||
import os |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused import
import os |
tmpdir, | ||
], | ||
) | ||
print(sp.returncode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove print here?
print(sp.returncode) |
@@ -88,6 +100,82 @@ def clean_docs(_session: Session) -> None: | |||
shutil.rmtree(docs_folder) | |||
|
|||
|
|||
@nox.session(name="docs:links", python=False) | |||
def docs_list_links(session: Session) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to create an integration test and/or unit test(s) similar to https://github.com/exasol/python-toolbox/blob/main/test/unit/util/dependencies/poetry_dependencies_test.py.
For the integration test:
- Within a tmp directory, make a simple set up with 1 to 3 links & test that what you capture on the capsys matches what you would expect.
For unit test(s):
- You could either run it over
docs_list_links
and mock the session like is done https://github.com/exasol/python-toolbox/blob/main/test/unit/nox/_artifacts_test.py or extract your for loop (line 124-136) & test it with hard-coded input like you would expect from the preceding lines.
It is true that we should not test everything that sphinx-build linkcheck does, but as we are building a small part of code (line 124-136) around it, it is best to ensure that the given certain input that we get an expected output. In the example test code (poetry_dependencies_test.py.
) above, this was necessitated as when we moved from poetry 1.x to poetry 2.x the format of the pyproject.toml
changed, but we had only hard-coded tests that represented a format that no longer existed. An analogous situation could be when we updated sphinx-build that we find the the JSONL format has changed or that the conf.py
variable names have changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(In the example with poetry, there was no CI indication before that the code was failing. I just happened to see that the output in the GitHub summary wasn't correct. 😬 . Better to do light testing & catch it earlier on. Tests are relatively free. Developers finding issues isn't as much; we cost more per hour 😉 )
formatter_class=argparse.ArgumentDefaultsHelpFormatter, | ||
) | ||
parser.add_argument( | ||
"-o", "--output", type=Path, help="path to output file", default=None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"-o", "--output", type=Path, help="path to output file", default=None | |
"-o", "--output", type=Path, help="path to copy the output json file", default=None |
shutil.copyfile(result_json, dst) | ||
print(f"file generated at path: {result_json.resolve()}") | ||
result_txt = tmpdir / "output.txt" | ||
if sp.returncode == 1 or result_txt.read_text() != "": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe something like this
-> separate errors visually with a character like -
-> use a walrus operator in some way so that not doing read_text() twice. If you use a walrus operator then, it should be the first item in the or condition, as otherwise, it may not be set later.
if sp.returncode == 1 or result_txt.read_text() != "": | |
if errors:=result_txt.read_text().splitlines() or sp.returncode == 1: | |
escape_red = "\033[31m" | |
print(escape_rot + "errors:") | |
print('\n'.join(f"- {item}" for item in errors)) | |
session.error(1) |
closes #409