Summary
Commit 2ee2192 in #7319 adds a hotfix in tests/conftest.py so caplog-based tests keep working after setting the top-level aiida logger to propagate = False.
The hotfix works by monkeypatching logging.Logger.handle for tests that request caplog.
This unblocks CI, but it relies on patching a stdlib logging internals method and should be replaced with a more robust solution.
Background
src/aiida/common/log.py now configures the top-level aiida logger with propagate: False.
That is desirable in production, but a lot of tests relied on pytest's caplog capturing records through the root logger. Once propagation was disabled, many existing assertions such as assert '...' in caplog.text started failing.
A first attempt to fix this by patching logging.Logger.handle globally caused unrelated failures in tests that fork worker processes:
tests/cmdline/commands/test_process.py::test_process_kill_failing_transport
tests/cmdline/commands/test_process.py::test_process_kill_failing_transport_failed_kill
tests/cmdline/commands/test_process.py::test_process_kill_failing_ebm_transport
tests/cmdline/commands/test_process.py::test_process_kill_failing_ebm_kill
Those failures were caused by pickling errors involving the patched Logger.handle.
The current hotfix avoids that by only applying the patch for tests that explicitly use caplog.
Problem
The current workaround is still fragile because it:
- monkeypatches
logging.Logger.handle, which is a core stdlib logging method
- depends on pytest fixture internals (
request.fixturenames / request.getfixturevalue('caplog'))
- could still interact badly with tests that reconfigure logging
- is harder to reason about than a solution based on supported logging APIs
Desired outcome
Keep the production logging behavior:
aiida.propagate = False
verdi.propagate = False
But provide a proper test-side solution so that:
- existing
caplog assertions continue to work
- no stdlib logging internals need to be monkeypatched
- multiprocessing / worker / daemon tests do not fail with pickling issues
- the fix is centralized and easy to understand
Possible directions
A proper fix could involve one of the following approaches:
- Add a dedicated pytest logging integration that attaches a capture handler to the relevant AiiDA loggers using supported logging APIs.
- Adjust test logging configuration so
caplog can see aiida / verdi records without changing production defaults.
- Introduce an AiiDA-specific helper fixture for log assertions and migrate the most affected tests to use it.
The preferred solution should avoid monkeypatching logging.Logger.handle.
Summary
Commit 2ee2192 in #7319 adds a hotfix in
tests/conftest.pysocaplog-based tests keep working after setting the top-levelaiidalogger topropagate = False.The hotfix works by monkeypatching
logging.Logger.handlefor tests that requestcaplog.This unblocks CI, but it relies on patching a stdlib logging internals method and should be replaced with a more robust solution.
Background
src/aiida/common/log.pynow configures the top-levelaiidalogger withpropagate: False.That is desirable in production, but a lot of tests relied on pytest's
caplogcapturing records through the root logger. Once propagation was disabled, many existing assertions such asassert '...' in caplog.textstarted failing.A first attempt to fix this by patching
logging.Logger.handleglobally caused unrelated failures in tests that fork worker processes:tests/cmdline/commands/test_process.py::test_process_kill_failing_transporttests/cmdline/commands/test_process.py::test_process_kill_failing_transport_failed_killtests/cmdline/commands/test_process.py::test_process_kill_failing_ebm_transporttests/cmdline/commands/test_process.py::test_process_kill_failing_ebm_killThose failures were caused by pickling errors involving the patched
Logger.handle.The current hotfix avoids that by only applying the patch for tests that explicitly use
caplog.Problem
The current workaround is still fragile because it:
logging.Logger.handle, which is a core stdlib logging methodrequest.fixturenames/request.getfixturevalue('caplog'))Desired outcome
Keep the production logging behavior:
aiida.propagate = Falseverdi.propagate = FalseBut provide a proper test-side solution so that:
caplogassertions continue to workPossible directions
A proper fix could involve one of the following approaches:
caplogcan seeaiida/verdirecords without changing production defaults.The preferred solution should avoid monkeypatching
logging.Logger.handle.