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

One test failing in test_utils.py when running pytest tests #1521

Open
mnjowe opened this issue Nov 20, 2024 · 3 comments · May be fixed by #1522
Open

One test failing in test_utils.py when running pytest tests #1521

mnjowe opened this issue Nov 20, 2024 · 3 comments · May be fixed by #1522
Labels
bug Something isn't working

Comments

@mnjowe
Copy link
Collaborator

mnjowe commented Nov 20, 2024

When running pytest tests in the local environment, test_utils fails with the following error:
FAILED tests/test_utils.py::test_logs_parsing - AssertionError: assert 'tlo.methods.demography' in {}

Interestingly, running pytest tests/test_utils.py alone works perfectly fine, and all tests pass. After a quick debug, it appears that the issue is caused by another test, test_logging.py. Specifically, when running pytest tests/test_logging.py tests/test_utils.py, the test_utils fails with the above error. I guess test_logging is modifying some global state, as running the tests in a different order (pytest tests/test_utils.py tests/test_logging.py) resolves the issue, and all tests pass successfully.

@tamuri or @matt-graham can you help looking into this?

@mnjowe mnjowe added the bug Something isn't working label Nov 20, 2024
@matt-graham
Copy link
Collaborator

matt-graham commented Nov 20, 2024

Hi @mnjowe. Is this running on current master (49acef5). Running

tox -e py311 -- pytest tests/test_utils.py::test_logs_parsing

locally or

pytest tests/test_utils.py::test_logs_parsing

both pass for me without any failures (as does just running pytest tests/test_utils.py).

EDIT: Ignore the above I misread your comment, hadn't realised this was appearing after running in conjuction with tests_logging.py

@matt-graham
Copy link
Collaborator

So I think this is happening because of the initialise_logging fixture which is (auto)used in the tests in tests/test_logging.py

@pytest.fixture(autouse=True)
def initialise_logging(
add_stdout_handler: bool,
simulation_date_getter: core.SimulationDateGetter,
root_level: core.LogLevel,
stdout_handler_level: core.LogLevel,
) -> Generator[None, None, None]:
logging.initialise(
add_stdout_handler=add_stdout_handler,
simulation_date_getter=simulation_date_getter,
root_level=root_level,
stdout_handler_level=stdout_handler_level,
)
yield
logging.reset()

This was intended to allow testing functions which alter the global logging state without coupling them together by resetting the global state to what is was at the (first) import of tlo.logging.core.

What I didn't account for in this is that the global logging state is affected by the various logger = logging.getLogger(__name__) and related statements in each of the tlo submodules which are executed when they are first imported, which happens after the first import of tlo.logging.core. This means this fixture is removing the side effects on the global logging state of importing these modules. Just removing the line

logging.reset()

so that the global state is not reset appears to resolve the test failure issue. Ideally it would be good to still allow explicitly resetting the global logging state to a previous value in tests, but I think a context manager solution for doing this might work better - that is the context manager records initial global state, yields to code in a with block which potentially alters global state, and then finalises by restoring global state to initial value. I will try get a PR opened to fix this today!

@mnjowe
Copy link
Collaborator Author

mnjowe commented Nov 20, 2024

Thanks Matt

@matt-graham matt-graham linked a pull request Nov 20, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants