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

Should we make the Docs CI mandatory on the Python main branch? #424

Closed
vstinner opened this issue Jan 17, 2022 · 9 comments
Closed

Should we make the Docs CI mandatory on the Python main branch? #424

vstinner opened this issue Jan 17, 2022 · 9 comments

Comments

@vstinner
Copy link
Member

Hi,

When I wanted to merge my fix GH fix, I saw that the Docs CI failed. After looking at the failure, I saw that it was unrelated to my change, but was caused by a recent enum documentation change: https://bugs.python.org/issue40066#msg410711

I reverted the enum change to merge my PR. The enum PR was merged even if the Docs CI failed. Well, the Docs job is not mandatory.

Should we make the Docs CI mandatory on the Python main branch? cc @tiran @zware

Is there a reason to not make the Docs CI mandatory? Was it flaky last months? cc @willingc @JulienPalard

By the way, who has access to the Python branch protection now? I used to ping Brett, but he wrote that he prefers to no longer be responsible for that. https://mail.python.org/archives/list/python-committers@python.org/thread/GB7T23XWLNKR24V5IWBADYSLFK6KWCY6/

Victor

@tiran
Copy link
Member

tiran commented Jan 17, 2022

I would make all GH CI jobs required.

@ethanfurman
Copy link
Member

My apologies for what may be a stupid question, but is there a way to run the doc tests on our local machines that matches what CI does? I spent several iterations trying to get the doc tests to pass on CI (each one passed on my local system), before giving up and merging the PR.

@vstinner
Copy link
Member Author

Optional jobs:

  • Windows (x86): time to time, test_asyncio fails on it: https://bugs.python.org/issue41682
  • macOS can take longer than 1h to run. It looks like GitHub Actions has limited resources on macOS.
  • Azure Pipelines fails sometimes with network failures, test_asyncio failures and a few others that I don't recall.

I would prefer to keep these jobs optional until they are faster and more reliable. For Windows x86, IMO having Windows x64 for now is enough.

@vstinner
Copy link
Member Author

My apologies for what may be a stupid question, but is there a way to run the doc tests on our local machines that matches what CI does?

Look into .github/workflows/doc.yml. It seems like currently, the commands are:

./configure --with-pydebug
make -j4
make -C Doc/ PYTHON=../python venv
make -C Doc/ PYTHON=../python SPHINXOPTS="-q -W --keep-going -j4" check
xvfb-run make -C Doc/ PYTHON=../python SPHINXOPTS="-q -W --keep-going -j4" doctest
make -C Doc/ PYTHON=../python SPHINXOPTS="-q -W --keep-going -j4" html

@vstinner
Copy link
Member Author

I spent several iterations trying to get the doc tests to pass on CI (each one passed on my local system), before giving up and merging the PR.

@tiran @JulienPalard @willingc: if I recall, correctly, it already happened to me that the Sphinx build succeed when using SPHINXOPTS="-jN", but it failed when this option was not used. Do you have such issue sometimes? Should we remove -j4 in the CI job to make it more reliable?

@Mariatta
Copy link
Member

Mariatta commented Jan 17, 2022

By the way, who has access to the Python branch protection now? 

The admins of the CPython repo is documented here:

https://devguide.python.org/devcycle/#repository-administration

@JulienPalard
Copy link
Member

I'm not a fan of having the doc CI to fail unnoticed, it breaks buildbots and may break builds on docs.python.org :(

I don't know if enforcing is a good thing (human should keep control), but it implies that we should not merge PRs when the CI fail without a very good reason.

Anyway if the doc CI is flacky, when you notice it, please open an issue (I am not aware of it being flacky but I don't have much bandwidth those days to monitor it).

@vstinner
Copy link
Member Author

Sadly, I forgot the issue that I had with "doctest" when I used SPHINXOPTS="-jN". Yeah, I can open a new ticket if such bug strikes back ;-)

@hugovk
Copy link
Member

hugovk commented Jan 17, 2024

After python/cpython#97533, docs builds are now required to pass on the CI:

For example python/cpython#112357 has:

📝 check-docs → ✓ success [required to succeed]

https://github.com/python/cpython/actions/runs/6979764201/job/18993877933?pr=112357

@hugovk hugovk closed this as completed Jan 17, 2024
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

No branches or pull requests

6 participants