#8711: Handle disabled logging in 'caplog.set_level' and 'caplog.at_level' #8758

alexlambson · 2021-06-13T06:24:53Z

Forces requested caplog logging levels to be enabled if they were disabled via logging.disable()

[attr-defined] mypy error ignored in logging.py because there were existing errors with the imports
and loggin.Logger.manager is an attr set at runtime. Since it's in the standard lib I can't really fix that.

Ignored an attr-defined error in src/_pytest/config/__init__.py because the re-export is necessary.

Closes #8711

alexlambson · 2021-06-13T06:26:54Z

src/_pytest/logging.py

+
+        :return int: The original disabled logging level.
+        """
+        original_disable_level: int = logger_obj.manager.disable  # type: ignore[attr-defined]


Linter doesn't like runtime attributes, so the ignore is needed to the best of my knowledge.

src/_pytest/logging.py

alexlambson · 2021-06-13T06:28:43Z

src/_pytest/logging.py

@@ -448,6 +488,9 @@ def set_level(self, level: Union[int, str], logger: Optional[str] = None) -> Non
        if self._initial_handler_level is None:
            self._initial_handler_level = self.handler.level
        self.handler.setLevel(level)
+        initial_disabled_logging_level = self.force_enable_logging(level, logger_obj)
+        if self._initial_disabled_logging_level is None:
+            self._initial_disabled_logging_level = initial_disabled_logging_level


We could remove this magic for a parameter if magic is frowned upon.

alexlambson · 2021-09-25T03:35:21Z

Just bumping / following up on this because it's popped up for me again in a project.

andrewdotn

Thanks for sending this in! I’m new here, so I hope my comments are helpful.

src/_pytest/logging.py

testing/logging/test_fixture.py

src/_pytest/logging.py

andrewdotn · 2021-10-31T05:37:52Z

src/_pytest/logging.py

    def set_level(self, level: Union[int, str], logger: Optional[str] = None) -> None:
        """Set the level of a logger for the duration of a test.

        .. versionchanged:: 3.4
            The levels of the loggers changed by this function will be
            restored to their initial values at the end of the test.

+        Will enable the requesed logging level if it was disabled via :meth:`logging.disable`.


Suggested change

Will enable the requesed logging level if it was disabled via :meth:`logging.disable`.

Will enable the requested logging level if it was disabled via :meth:`logging.disable`.

I guess my "t" finger was tired that day.

alexlambson · 2021-11-03T06:22:37Z

Thanks for sending this in! I’m new here, so I hope my comments are helpful.

Thanks for the review. I'll fix these in the morning :)

- Resolves issues from review provided by https://github.com/andrewdotn - Fixed spelling errors - Improved logic in `force_enable_logging` - Make a fixture to un-disable logging after a test so that logging will be un-disabled even if the test fails due to an assertion error. The fixture is in `test_fixture.py` because we can't `import logging` in `conftest.py` because there is a module called `logging` in the same folder. - Mypy implicit import error. I can't commit without this. Issue: pytest-dev#8711 PR: pytest-dev#8758

alexlambson · 2021-11-06T03:07:59Z

The code coverage seems to not look into those string-based tests. Most of the untested lines are actually tested. Should I just write direct function tests?

- Adds test coverage for `force_enable_logging` with a string `level`. Fixes [code coverage check](https://github.com/pytest-dev/pytest/pull/8758/checks?check_run_id=4123920877) Issue: pytest-dev#8711 PR: pytest-dev#8758

alexlambson · 2021-11-11T06:34:05Z

Fixed test coverage

blerrgh · 2023-05-17T01:42:34Z

What happened to this? Would be a super handy feature to have.

alexlambson · 2023-05-17T01:43:30Z

Mainly just waiting for approval

nicoddemus

Hi @alexlambson,

Really sorry this fell through the tracks and got forgotten for so long, I appreciate the recent pings.

I left a few minor comments, the PR is really well written, including the tests.

If you could just rebase it and fix the comments I left, I will be glad to get this merged.

👍

src/_pytest/logging.py

testing/logging/test_fixture.py

src/_pytest/logging.py

nicoddemus

Hi @alexlambson,

Really sorry this fell through the tracks and got forgotten for so long, I appreciate the recent pings.

I left a few minor comments, the PR is really well written, including the tests.

If you could just rebase it and fix the comments I left, I will be glad to get this merged.

👍

…ytest-dev#8711 Forces requested `caplog` logging levels to be enabled if they were disabled via `logging.disable()` `[attr-defined]` mypy error ignored in `logging.py` because there were existing errors with the imports and `loggin.Logger.manager` is an attr set at runtime. Since it's in the standard lib I can't really fix that. Ignored an attr-defined error in `src/_pytest/config/__init__.py` because the re-export is necessary. Issue: pytest-dev#8711

- Resolves issues from review provided by https://github.com/andrewdotn - Fixed spelling errors - Improved logic in `force_enable_logging` - Make a fixture to un-disable logging after a test so that logging will be un-disabled even if the test fails due to an assertion error. The fixture is in `test_fixture.py` because we can't `import logging` in `conftest.py` because there is a module called `logging` in the same folder. - Mypy implicit import error. I can't commit without this. Issue: pytest-dev#8711 PR: pytest-dev#8758

- Adds test coverage for `force_enable_logging` with a string `level`. Fixes [code coverage check](https://github.com/pytest-dev/pytest/pull/8758/checks?check_run_id=4123920877) Issue: pytest-dev#8711 PR: pytest-dev#8758

- Address review ad rebase to latest from main - Make `force_enable_logging` private. Issue: pytest-dev#8711 PR: pytest-dev#8758

alexlambson · 2023-05-18T03:49:22Z

No worries @nicoddemus, pytest is a busy project.

~~I am curious why that diff pipeline is failing? I covered my code fairly well I thought.~~ nvm, it was thinking.

nicoddemus

Thanks a lot @alexlambson!

alexlambson mentioned this pull request Jun 13, 2021

Handle logging.disable() in caplog.set_level and caplog.at_level #8711

Closed

alexlambson commented Jun 13, 2021

View reviewed changes

src/_pytest/logging.py Outdated Show resolved Hide resolved

alexlambson commented Jun 13, 2021

View reviewed changes

andrewdotn reviewed Oct 31, 2021

View reviewed changes

alexlambson force-pushed the issue-8711-handle-disabled-logging branch from ec6f9d0 to 3537eaf Compare November 6, 2021 01:02

alexlambson force-pushed the issue-8711-handle-disabled-logging branch from 377d826 to e52ee7c Compare November 11, 2021 05:49

nicoddemus requested changes May 17, 2023

View reviewed changes

src/_pytest/logging.py Outdated Show resolved Hide resolved

src/_pytest/logging.py Outdated Show resolved Hide resolved

testing/logging/test_fixture.py Show resolved Hide resolved

src/_pytest/logging.py Outdated Show resolved Hide resolved

nicoddemus requested changes May 17, 2023

View reviewed changes

Alex Lambson and others added 4 commits May 17, 2023 21:25

caplog un-disable logging, add missing test coverage

150914d

- Adds test coverage for `force_enable_logging` with a string `level`. Fixes [code coverage check](https://github.com/pytest-dev/pytest/pull/8758/checks?check_run_id=4123920877) Issue: pytest-dev#8711 PR: pytest-dev#8758

caplog un-disable logging, add missing test coverage

7f89996

- Address review ad rebase to latest from main - Make `force_enable_logging` private. Issue: pytest-dev#8711 PR: pytest-dev#8758

alexlambson force-pushed the issue-8711-handle-disabled-logging branch from e52ee7c to 7f89996 Compare May 18, 2023 03:42

nicoddemus approved these changes May 18, 2023

View reviewed changes

nicoddemus merged commit ba32a3b into pytest-dev:main May 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#8711: Handle disabled logging in 'caplog.set_level' and 'caplog.at_level' #8758

#8711: Handle disabled logging in 'caplog.set_level' and 'caplog.at_level' #8758

alexlambson commented Jun 13, 2021

alexlambson Jun 13, 2021

alexlambson Jun 13, 2021

alexlambson commented Sep 25, 2021

andrewdotn left a comment

andrewdotn Oct 31, 2021

alexlambson Nov 3, 2021

alexlambson Nov 6, 2021

alexlambson commented Nov 3, 2021

alexlambson commented Nov 6, 2021 •

edited

Loading

alexlambson commented Nov 11, 2021

blerrgh commented May 17, 2023

alexlambson commented May 17, 2023

nicoddemus left a comment

nicoddemus left a comment

alexlambson commented May 18, 2023 •

edited

Loading

nicoddemus left a comment

	Will enable the requesed logging level if it was disabled via :meth:`logging.disable`.
	Will enable the requested logging level if it was disabled via :meth:`logging.disable`.

#8711: Handle disabled logging in 'caplog.set_level' and 'caplog.at_level' #8758

#8711: Handle disabled logging in 'caplog.set_level' and 'caplog.at_level' #8758

Conversation

alexlambson commented Jun 13, 2021

alexlambson Jun 13, 2021

Choose a reason for hiding this comment

alexlambson Jun 13, 2021

Choose a reason for hiding this comment

alexlambson commented Sep 25, 2021

andrewdotn left a comment

Choose a reason for hiding this comment

andrewdotn Oct 31, 2021

Choose a reason for hiding this comment

alexlambson Nov 3, 2021

Choose a reason for hiding this comment

alexlambson Nov 6, 2021

Choose a reason for hiding this comment

alexlambson commented Nov 3, 2021

alexlambson commented Nov 6, 2021 • edited Loading

alexlambson commented Nov 11, 2021

blerrgh commented May 17, 2023

alexlambson commented May 17, 2023

nicoddemus left a comment

Choose a reason for hiding this comment

nicoddemus left a comment

Choose a reason for hiding this comment

alexlambson commented May 18, 2023 • edited Loading

nicoddemus left a comment

Choose a reason for hiding this comment

alexlambson commented Nov 6, 2021 •

edited

Loading

alexlambson commented May 18, 2023 •

edited

Loading