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

Add Sphinx-compatible docstrings and inline comments for main.py (lines 245-490) #13091

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions changelog/12979.doc.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
* Added Sphinx-compatible docstrings and inline comments for functions in
`pytest/src/_pytest/main.py` (lines 245-490).
86 changes: 72 additions & 14 deletions src/_pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,24 +248,37 @@ def pytest_addoption(parser: Parser) -> None:


def validate_basetemp(path: str) -> str:
# GH 7119
"""
Validate the base temporary directory path.
Copy link
Contributor

@dongfangtianyu dongfangtianyu Dec 31, 2024

Choose a reason for hiding this comment

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

GH 7119 refers to #7119 clarifying the origin of the code, suggest keeping it in the docstrings, for example:

Validate the base temporary directory path (see also :issue:`7119`).


Args:
path (str): The path to validate.

Returns:
str: The validated path.

Raises:
argparse.ArgumentTypeError: If the path is invalid, such as being empty, the
current working directory, or a parent directory of the current working directory.
"""
# Error message for invalid paths
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Error message for invalid paths

msg = "basetemp must not be empty, the current working directory or any parent directory of it"

# empty path
# Check for an empty path
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Check for an empty path

if not path:
raise argparse.ArgumentTypeError(msg)

def is_ancestor(base: Path, query: Path) -> bool:
"""Return whether query is an ancestor of base."""
"""Check if the query path is an ancestor of the base path."""
if base == query:
return True
return query in base.parents

# check if path is an ancestor of cwd
# Ensure the path is not an ancestor of the current working directory
if is_ancestor(Path.cwd(), Path(path).absolute()):
raise argparse.ArgumentTypeError(msg)

# check symlinks for ancestors
# Check symlinks for ancestral relationships
if is_ancestor(Path.cwd().resolve(), Path(path).resolve()):
raise argparse.ArgumentTypeError(msg)

Expand All @@ -275,17 +288,26 @@ def is_ancestor(base: Path, query: Path) -> bool:
def wrap_session(
config: Config, doit: Callable[[Config, Session], int | ExitCode | None]
) -> int | ExitCode:
"""Skeleton command line program."""
"""
Execute the pytest session within a controlled wrapper.
Copy link
Contributor

Choose a reason for hiding this comment

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

The purpose is to execute doit, not to execute session,
Consider modifying to:

Execute a function within a pytest session.


Args:
config (Config): The pytest configuration object.
doit (Callable): Function defining the session execution logic.

Returns:
int | ExitCode: The exit status of the session.
"""
session = Session.from_config(config)
session.exitstatus = ExitCode.OK
initstate = 0
try:
try:
config._do_configure()
config._do_configure() # Initialize pytest configuration
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
config._do_configure() # Initialize pytest configuration
config._do_configure()

initstate = 1
config.hook.pytest_sessionstart(session=session)
config.hook.pytest_sessionstart(session=session) # Notify start of session
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
config.hook.pytest_sessionstart(session=session) # Notify start of session
config.hook.pytest_sessionstart(session=session)

initstate = 2
session.exitstatus = doit(config, session) or 0
session.exitstatus = doit(config, session) or 0 # Execute main logic
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
session.exitstatus = doit(config, session) or 0 # Execute main logic
session.exitstatus = doit(config, session) or 0

Copy link
Contributor

Choose a reason for hiding this comment

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

now,execute doit, not execute main. (These line comments may be unnecessary because the method names are already intuitive enough)

except UsageError:
session.exitstatus = ExitCode.USAGE_ERROR
raise
Expand Down Expand Up @@ -313,11 +335,10 @@ def wrap_session(
else:
if isinstance(excinfo.value, SystemExit):
sys.stderr.write("mainloop: caught unexpected SystemExit!\n")

finally:
# Explicitly break reference cycle.
# Explicitly break reference cycles
excinfo = None # type: ignore
os.chdir(session.startpath)
os.chdir(session.startpath) # Return to the initial directory
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
os.chdir(session.startpath) # Return to the initial directory
os.chdir(session.startpath)

if initstate >= 2:
try:
config.hook.pytest_sessionfinish(
Expand All @@ -332,12 +353,30 @@ def wrap_session(


def pytest_cmdline_main(config: Config) -> int | ExitCode:
"""
The main entry point for pytest command-line execution.

Args:
config (Config): The pytest configuration object.

Returns:
int | ExitCode: The exit status of the pytest session.
"""
return wrap_session(config, _main)


def _main(config: Config, session: Session) -> int | ExitCode | None:
"""Default command line protocol for initialization, session,
running tests and reporting."""
"""
Default command line protocol for initialization, session,
running tests, and reporting.

Args:
config (Config): The pytest configuration object.
session (Session): The pytest session object.

Returns:
int | ExitCode | None: The exit status of the session or None.
"""
config.hook.pytest_collection(session=session)
config.hook.pytest_runtestloop(session=session)

Expand All @@ -349,10 +388,29 @@ def _main(config: Config, session: Session) -> int | ExitCode | None:


def pytest_collection(session: Session) -> None:
"""
Perform the test collection phase for the given pytest session.

Args:
session (Session): The pytest session object.
"""
session.perform_collect()


def pytest_runtestloop(session: Session) -> bool:
"""
Execute the test run loop for the given pytest session.

Args:
session (Session): The pytest session object.

Returns:
bool: True if the loop completes successfully, False otherwise.

Raises:
session.Interrupted: If the test session is interrupted.
session.Failed: If the session encounters a failure condition.
"""
if session.testsfailed and not session.config.option.continue_on_collection_errors:
raise session.Interrupted(
f"{session.testsfailed} error{'s' if session.testsfailed != 1 else ''} during collection"
Expand Down
Loading