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

Update advice about _called_from_test. #6168

Merged
merged 1 commit into from Nov 14, 2019
Merged

Update advice about _called_from_test. #6168

merged 1 commit into from Nov 14, 2019

Conversation

shields-fn
Copy link
Contributor

Instead of giving an example of using sys and then, at the end, advising to not use sys, just give a preferred example. This is especially helpful since mypy 0.740 has started (correctly) complaining about sys._called_from_test not being present.

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.

as we dont have a executing example yet this is fine as is, but we should follow up with an actual execution at a later point

doc/en/example/simple.rst Show resolved Hide resolved

.. code-block:: python

if hasattr(sys, "_called_from_test"):
if your_module._called_from_test:
# called from within a test run
...
else:
# called "normally"
...

Copy link
Member

Choose a reason for hiding this comment

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

i noted that we haven't yet a actual example execution block, so this example isn't actually executed demoing the effect -

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do I do that?

Copy link
Member

Choose a reason for hiding this comment

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

TBH I think the current example is fine, it is a really contrived use case anyways. 🤷‍♂

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

I think this is mergeable as is but will defer clicking the button until @RonnyPfannschmidt has had additional input 👍

@nicoddemus
Copy link
Member

We should squash the commits before merging as well.

Instead of giving an example of using sys and then, at the end,
advising not to use sys, just give a correct example. This is
especially helpful since mypy 0.740 has started (correctly) complaining
about sys._called_from_pytest not being present.
@RonnyPfannschmidt
Copy link
Member

please merge, having it as runner example take a bit of tinkering

@nicoddemus nicoddemus merged commit e2a0987 into pytest-dev:master Nov 14, 2019
@shields-fn shields-fn deleted the patch-1 branch November 14, 2019 20:17
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

4 participants