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

Updated docs: Now we can use environment variable PYTEST_VERSION to detect if a code is run by a pytest run #12153

Merged
merged 8 commits into from Apr 28, 2024

Conversation

dheerajck
Copy link
Contributor

@dheerajck dheerajck commented Mar 22, 2024

Add a better way to detect if a code is running from within a pytest run.

@dheerajck
Copy link
Contributor Author

#9502 (comment)

@dheerajck dheerajck changed the title Update docs: Add a better way to detect if a code is running from within a pytest run Update docs: Documented a better way to detect if a code is running from within a pytest run Mar 22, 2024
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

The import check is actually a more fragile version a it also triggers whenever valid pytest Import was done

I recommend listing the import check as naive and the Hook based one as robust

@dheerajck dheerajck changed the title Update docs: Documented a better way to detect if a code is running from within a pytest run Updated docs: Documented two ways to detect if a code is running from within a pytest run. Mar 22, 2024
@dheerajck dheerajck changed the title Updated docs: Documented two ways to detect if a code is running from within a pytest run. Updated docs: Documented two ways to detect if a code is running from within a pytest run Mar 22, 2024
@dheerajck
Copy link
Contributor Author

Also sorry for too much force pushes

@dheerajck
Copy link
Contributor Author

dheerajck commented Mar 22, 2024

Converting to draft to see if there is a better way to check if a code is running from within pytest.

@dheerajck dheerajck marked this pull request as draft March 22, 2024 22:07
@nicoddemus
Copy link
Member

Thanks @dheerajck, but I think the docs as is already shows a robust solution, why show an alternative (checking "pytest" in sys.modules) if the approach using pytest_configure is for all purposes better?

@dheerajck
Copy link
Contributor Author

I dont want to add "pytest" in sys.modules as @RonnyPfannschmidt pointed out an issue with it, thats why I converted this PR to draft to check if theres a better approach

The approach in doc doesnt work in every case, this issue mentions one case where it doesnt work

pytest documents a pattern to change behaviour using fixtures here: https://docs.pytest.org/en/latest/example/simple.html#detect-if-running-from-within-a-pytest-run . Unfortunately this techinque is incompatible with this particular problem. pytest-django's fixtures load the settings module early, before fixtures from conftest.py fixtures run, so the pattern cannot be applied. And environment variable changes cannot be undone after the fact.

I think we should add an env for this purpose like mentioned in that issue by @The-Compiler

@dheerajck
Copy link
Contributor Author

if maintainers would like to have that I can create a PR for that
#9502 (comment)

@nicoddemus

@nicoddemus
Copy link
Member

if maintainers would like to have that I can create a PR for that

👍 by me!

I understand the concerns from @RonnyPfannschmidt in #9502 (comment):

Personally im -1 on adding easy and obvious fast paths to make it easy to keep messy global configuration around

While I agree in principle to avoid adding easy ways for people to do the wrong thing, in this case I think we should add this because:

  1. Often people are stuck with previous design decisions (like importing environment variables during import) and they have no control over, they are just trying to do the best for their users and the projects without breaking the world.
  2. It is trivial for us to add, no backward compatibility concerns.

For this reason I think we should go for it.

@RonnyPfannschmidt
Copy link
Member

In that case I propose introducing a get-current-testrun-state kind of helper that return details like current item, current phase, stats as immutable entity
Exact names and details will need bike shedding

@nicoddemus
Copy link
Member

But one of the requirements is to not having to import pytest at all, this goes against that.

@dheerajck
Copy link
Contributor Author

yes, I would suggest to not force importing pytest in non ut codes
and configvar probably will not help when we use pytest xdist parallel workers for example
if we are adding a method to check if a code is run by pytest, I prefer it to work with all cases

I have added a pr for this change #12190
and if its merged after fixing / changing things, I will update this pr to update docs

@nicoddemus
Copy link
Member

@dheerajck thanks for updating it.

But now that we have PYTEST_VERSION, wouldn't be better to just advertise that solution, is it has no downsides?

@dheerajck
Copy link
Contributor Author

yes, I will update the docs with new changes

@dheerajck
Copy link
Contributor Author

Hi sorry, kinda busy as I started to look for a new job

@dheerajck
Copy link
Contributor Author

Done @nicoddemus

changelog/12153.doc.rst Outdated Show resolved Hide resolved
doc/en/example/simple.rst Outdated Show resolved Hide resolved
doc/en/example/simple.rst Outdated Show resolved Hide resolved
@nicoddemus nicoddemus marked this pull request as ready for review April 28, 2024 17:08
@nicoddemus
Copy link
Member

Thanks @dheerajck!

@nicoddemus nicoddemus enabled auto-merge (squash) April 28, 2024 17:10
@nicoddemus nicoddemus merged commit 2ede877 into pytest-dev:main Apr 28, 2024
24 checks passed
@dheerajck dheerajck changed the title Updated docs: Documented two ways to detect if a code is running from within a pytest run Updated docs: Now we can use environment variable PYTEST_VERSION to detect if a code is run by a pytest run Apr 28, 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

Successfully merging this pull request may close these issues.

None yet

3 participants