details impact of specifying CI or BUILD_NUMBER #12578
Conversation
Indeed, we have made steps towards improving this recently. |
doc/en/reference/reference.rst
Outdated
| @@ -1120,7 +1120,11 @@ Environment variables that can be used to change pytest's behavior. | |||
|
|
|||
| .. envvar:: CI | |||
|
|
|||
| When set (regardless of value), pytest acknowledges that is running in a CI process. Alternative to ``BUILD_NUMBER`` variable. | |||
| When set (regardless of value), pytest acknowledges that is running in a CI process: | |||
There was a problem hiding this comment.
It would probably be best for us to create a new docs session somewhere detailing how pytest detects CI and the consequences of that, and link to that session here.
There was a problem hiding this comment.
Took a cursory look and did not find a suitable place to add this information to, perhaps we can just create a new document in doc/en/explanation/ci.rst. This could explain the rationale for auto-detecting CI, mention the two variables, and what pytest does differently when running on CI.
There was a problem hiding this comment.
I feel like "how we detect we are in a CI env?" and "what are the consequences?" are too short and simple to have a doc session (which I guess is a dedicated file?) somewhere
There was a problem hiding this comment.
I disagree, I don't think it is a problem having a short document: it is a place to explain this rationale, and to grow in case we change other behaviors in the future depending if we are on CI or not.
There was a problem hiding this comment.
fair enough! I'll try to dig out the original issue to create a good documentation :)
There was a problem hiding this comment.
Awesome, thanks! Really appreciate it.
I think a short document with:
- Rationale
- How CI is detected
- Effects on CI
With one or two paragraphs each, would be perfect.
psf-chronographer
bot
added
the
bot:chronographer:provided
|
the output of |
Do not worry about it, we update it at release time. |
|
should I drop my commit then ? |
|
Ahh no harm keeping it. 👍 😁 |
There was a problem hiding this comment.
Thanks @MarcBresson this looks great!
Applied some syntax/grammar fixes, but minor ones in general.
LGTM for merging, but will leave it open for a few days in case others want to chime in (specially @The-Compiler which identified the need initially).
Backport to 8.2.x: 💚 backport PR created✅ Backport PR branch: Backported as #12587 🤖 @patchback |
Closes #12577 Co-authored-by: Marc Bresson <[email protected]> Co-authored-by: Bruno Oliveira <[email protected]> (cherry picked from commit 6933bef)
Closes #12577 Co-authored-by: Marc Bresson <[email protected]> Co-authored-by: Bruno Oliveira <[email protected]> (cherry picked from commit 6933bef) Co-authored-by: Marc Bresson <[email protected]>
I also added the mention in
pytest -h. Closes #12577.FYI, the doc of 8.x is not very well referenced, and when I was looking for this env var, I always was on the doc of version 7.x