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

Make pytest.skip work in doctest #4927

Merged
merged 5 commits into from
Mar 15, 2019
Merged

Conversation

tkf
Copy link
Contributor

@tkf tkf commented Mar 15, 2019

As discussed in #4911 (comment), it would be nice if doctest can be conditionally skipped. It seems that just adding three lines is enough (close #4911). All this patch does is throwing Skipped instead of UnexpectedException if the original exception is Skipped.

I'll add tests and docs if this direction looks OK.


  • Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
  • Target the master branch for bug fixes, documentation updates and trivial changes.
  • Target the features branch for new features and removals/deprecations.
  • Include documentation when adding new features.
  • Include new tests or update existing tests when applicable.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

  • Add yourself to AUTHORS in alphabetical order;

Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

Looks good to me in general.

@tkf
Copy link
Contributor Author

tkf commented Mar 15, 2019

Thanks. I added a test and some lines in docstring. I think it is ready for review.

changelog/4911.feature.rst Outdated Show resolved Hide resolved
@@ -80,7 +80,8 @@ def skip(msg="", **kwargs):
Skip an executing test with the given message.

This function should be called only during testing (setup, call or teardown) or
during collection by using the ``allow_module_level`` flag.
during collection by using the ``allow_module_level`` flag. This function can
be called in doctest as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
be called in doctest as well.
be called in doctests as well.

But not sure if it is useful to mention doctests here explicitly..?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm OK either way. I thought to mention it since doctests are special enough for how they are executed but yet very popular.

src/_pytest/outcomes.py Show resolved Hide resolved
Co-Authored-By: tkf <takafumi.a@gmail.com>
Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

Thanks!

src/_pytest/outcomes.py Outdated Show resolved Hide resolved
Co-Authored-By: tkf <takafumi.a@gmail.com>
@asottile asottile merged commit 5f52d5e into pytest-dev:features Mar 15, 2019
@tkf
Copy link
Contributor Author

tkf commented Mar 15, 2019

Thanks a lot!

@tkf tkf deleted the skip-doctest branch March 15, 2019 07:25
@asottile
Copy link
Member

Thanks a lot!

no, thanks you! good patch :D

@nicoddemus
Copy link
Member

Awesome @tkf! Glad I was incorrect in my first assessment then! 😁

@ihnorton ihnorton mentioned this pull request Apr 12, 2019
3 tasks
@pytest-dev pytest-dev deleted a comment from codecov bot Feb 3, 2020
@blueyed
Copy link
Contributor

blueyed commented Feb 3, 2020

For reference: #6669 adds it for all OutcomeExceptions (xfail, importorskip).

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.

4 participants