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

#7336 make --log-file relative to inifile if it exists #7350

Closed
Closed
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions changelog/7336.breaking.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The pytest ``--log-file`` cli or ``log_file`` ini marker is now relative to the configs ``inifile`` directory, as it was always the intention. It was originally introduced as relative to the current working directory unintentionally.
2 changes: 1 addition & 1 deletion src/_pytest/config/findpaths.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def determine_setup(
args: List[str],
rootdir_cmd_arg: Optional[str] = None,
config: Optional["Config"] = None,
) -> Tuple[py.path.local, Optional[str], Dict[str, Union[str, List[str]]]]:
) -> Tuple[py.path.local, Optional[py.path.local], Dict[str, Union[str, List[str]]]]:
Copy link
Member

Choose a reason for hiding this comment

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

Please make sure to convert inifile to a py.path.local inside if inifile: below to honor this type annotation.

Copy link
Member

Choose a reason for hiding this comment

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

I have a PR locally which addresses these issues (by fixing errors after running mypy with pytest-dev/py#232). I'll post it in a bit.

Copy link
Member Author

@symonk symonk Jun 12, 2020

Choose a reason for hiding this comment

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

test_inifilename hates such change(s) - investigating; maybe the test should be just updated:

'../../foo/bar.ini'  # becomes:
C:\Users\sy\AppData\Local\Temp\pytest-of-sy\pytest-972\test_inifilename0\foo\bar.ini 

rootdir = None
dirs = get_dirs_from_args(args)
if inifile:
Expand Down
4 changes: 4 additions & 0 deletions src/_pytest/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,10 @@ def __init__(self, config: Config) -> None:
# File logging.
self.log_file_level = get_log_level_for_setting(config, "log_file_level")
log_file = get_option_ini(config, "log_file") or os.devnull

# Keep the log file relative to the inidir file (if it exists) (#7336)
if log_file != os.devnull and config.inifile is not None:
log_file = os.path.join(config.inifile.dirname, log_file)
self.log_file_handler = _FileHandler(log_file, mode="w", encoding="UTF-8")
log_file_format = get_option_ini(config, "log_file_format", "log_format")
log_file_date_format = get_option_ini(
Expand Down
84 changes: 84 additions & 0 deletions testing/logging/test_reporting.py
Original file line number Diff line number Diff line change
Expand Up @@ -1152,3 +1152,87 @@ def test_bad_log(monkeypatch):
)
result = testdir.runpytest()
result.assert_outcomes(passed=1)


def test_log_file_is_in_pytest_ini_rootdir(testdir, monkeypatch):
"""
#7336|#7350 - log_file should be relative to the config inifile
"""
testdir.makefile(
Copy link
Member

Choose a reason for hiding this comment

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

No need to change, but you can also use makeinifile. 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

ah i got confused as it made a 'tox' file :)

Copy link
Member

Choose a reason for hiding this comment

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

Probably for historical reasons.

".ini",
pytest="""
[pytest]
log_cli = True
log_cli_level = DEBUG
log_file = logfile.txt
log_file_level = DEBUG
""",
)
sub = testdir.mkdir("sub")
p = testdir.makepyfile(
"""
def test_this():
import logging
logging.getLogger().info("Normal message")
"""
)
p.move(sub.join(p.basename))
monkeypatch.chdir(sub.strpath)
testdir.runpytest()
testdir.chdir()
files = set(os.listdir())
assert {"logfile.txt", "pytest.ini"}.issubset(files)


def test_log_file_can_be_specified_to_child_dir(testdir, monkeypatch):
"""
#7336|#7350 - log_file should be relative to the config inifile
"""
testdir.makefile(
".ini",
pytest="""
[pytest]
log_cli = True
log_cli_level = DEBUG
log_file = sub/logfile.txt
log_file_level = DEBUG
""",
)
sub = testdir.mkdir("sub")
p = testdir.makepyfile(
"""
def test_this():
pass
"""
)
p.move(sub.join(p.basename))
monkeypatch.chdir(sub.strpath)
testdir.runpytest()
files = set(os.listdir())
assert "logfile.txt" in files
assert "pytest.ini" not in files


def test_log_file_cli_is_also_relative(testdir, monkeypatch):
"""
#7336|#7350 - log_file should be relative to the config inifile
"""
testdir.makefile(
".ini",
pytest="""
[pytest]
""",
)
sub = testdir.mkdir("sub")
p = testdir.makepyfile(
"""
def test_this():
pass
"""
)
p.move(sub.join(p.basename))
monkeypatch.chdir(sub.strpath)
testdir.runpytest("--log-file", "sub{}logfile.txt".format(os.sep))
files = set(os.listdir())
assert "logfile.txt" in files
assert "pytest.ini" not in files