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.

Sign up for GitHub

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

Jump to bottom

Add an ini option doctest_standalone_namespace #6978

Closed
wants to merge 1 commit into from
Closed

Add an ini option doctest_standalone_namespace #6978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/_pytest/doctest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ def pytest_addoption(parser):
parser.addini(
"doctest_encoding", "encoding used for doctest files", default="utf-8"
)
parser.addini(
"doctest_standalone_namespace",
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't think of any better options for the name either.
How about calling it doctest_context with a non bool type, or even better, doctest_reset_context? That would leave the option up to implement something similar to the plot directive's context. And then, this can remain a bool.

Copy link
Member

Choose a reason for hiding this comment

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

I'm OK with doctest_reset_context, although perhaps doctest_empty_context better conveys what the option actually does?

Copy link
Author

Choose a reason for hiding this comment

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

From a technical point of view, "namespace" is a better term than "context". Two function doctests don't interact with each other, which is what "reset context" seems to imply to me. So if you define a variable in one doctest it won't be available in another doctest.

Rather, they just reuse the module namespace, so if a variable is defined at the module level (not in a doctest), it gets defined automatically in a doctest. Maybe a name like doctest_no_module_namespace (although honestly my biggest issue with the current name is that it is too long).

Copy link
Member

Choose a reason for hiding this comment

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

IMHO a long option name is OK if it correctly describes the behavior, because it will be written in a pytest.ini file only once anyway. 😁

Copy link
Author

Choose a reason for hiding this comment

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

I guess my problem isn't so much the length as the fact that the option name seems convoluted. I don't think many people will look at "doctest_standalone_namespace" and understand what it means unless they already know.

Copy link
Member

Choose a reason for hiding this comment

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

Fair point, agree.

doctest_no_module_namespace seems better in that light then, IMHO.

help="treat each module as having a standalone namespace",
type="bool",
default=False,
)
group = parser.getgroup("collect")
group.addoption(
"--doctest-modules",
Expand Down Expand Up @@ -465,15 +471,19 @@ def _find_lineno(self, obj, source_lines):
return doctest.DocTestFinder._find_lineno(self, obj, source_lines)

def _find(
self, tests, obj, name, module, source_lines, globs, seen
_self, tests, obj, name, module, source_lines, globs, seen
) -> None:
if _is_mocked(obj):
return

if self.config.getini("doctest_standalone_namespace"):
globs = {}
Copy link

Choose a reason for hiding this comment

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

Based on https://github.com/pytest-dev/pytest/blob/5.3.1/src/_pytest/doctest.py#L109-L113 that seems to use DoctestModule for *.py and DoctestTextfile for *.txt/*.rst, as this code change is only in DoctestModule, does this mean that this flag would apply only to the doctest examples inside *.py files and not the files that are fully doctest e.g. *.rst? Is that intended?

Copy link
Author

Choose a reason for hiding this comment

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

I don't think non-py files can have variables that are defined outside of the doctest. Or is there something I'm missing?

Copy link

Choose a reason for hiding this comment

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

There is the fixture doctest_namespace.

I am still getting familiar with it, though I have the impression that doctest_namespace carries state across modules / folders and I was wondering whether this patch was going to interact with doctest_namespace.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure how this currently interacts with that, nor how it should.

Copy link
Member

Choose a reason for hiding this comment

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

It does interact, but in this case I don't think the new option should interact at all with the doctest_namespace fixture: the intent there is to inject globals into the tests (such as the numpy import), so resetting that would defeat the purpose.

Copy link
Author

Choose a reason for hiding this comment

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

Maybe it makes sense to allow globs to be modified in a fixture in conftest.py (side note: "globs" is an unfortunate name from doctest; it is short for "globals" and has nothing to do with shell globs). Although I do like the simplicity of an ini setting to get the globs = {} behavior.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, taking a second look at the code, doctest_namespace won't be affected by this at all, because the doctest_namespace dict is injected during the setup phase of the test, while here globs is cleared during collection.

@asmeurer could you please add a test which ensures setting doctest_standalone_namespace to True still allow doctests to access the dict returned by doctest_namespace? We also should explicitly mention this in the description of the doctest_standalone_namespace.


with _patch_unwrap_mock_aware():

# Type ignored because this is a private function.
doctest.DocTestFinder._find( # type: ignore
self, tests, obj, name, module, source_lines, globs, seen
_self, tests, obj, name, module, source_lines, globs, seen
)

if self.fspath.basename == "conftest.py":
Expand Down
31 changes: 31 additions & 0 deletions testing/test_doctest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,37 @@ def test_encoding(self, testdir, test_string, encoding):

result.stdout.fnmatch_lines(["*1 passed*"])

@pytest.mark.parametrize("test_string", ["True", "False", None])
def test_standalone_namespace(self, testdir, test_string):
if test_string is not None:
testdir.makeini(
"""
[pytest]
doctest_standalone_namespace = {}
""".format(
test_string
)
)
testdir.makepyfile(
"""
def func():
'''
>>> func()
1
'''
return 1
"""
)
result = testdir.runpytest("--doctest-modules")

if test_string in ["False", None]:
result.stdout.fnmatch_lines(["*1 passed*"])
result.stdout.no_fnmatch_line("*FAILED*")
else:
result.stdout.no_fnmatch_line("*1 passed*")
result.stdout.fnmatch_lines("*FAILED*")
result.stdout.fnmatch_lines("NameError: name 'func' is not defined")

def test_doctest_unexpected_exception(self, testdir):
testdir.maketxtfile(
"""
Expand Down