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

Tests: Suppress logging and warnings from temporary profile fixture #5702

Merged

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Oct 18, 2022

When the test suite is run without specifying a particular test profile, one is created on the fly using the TemporaryProfileManager. This would create a temporary configuration directory and a temporary profile which would generate a UserWarning and some REPORT level log messages.

This info causes unnecessary noise and the warning even causes pytest to record that the tests issued a warning, but the warning is completely to be expected and benign, so here we capture the warnings and log messages.

When the test suite is run without specifying a particular test profile,
one is created on the fly using the `TemporaryProfileManager`. This
would create a temporary configuration directory and a temporary profile
which would generate a `UserWarning` and some `REPORT` level log
messages.

This info causes unnecessary noise and the warning even causes `pytest`
to record that the tests issued a warning, but the warning is completely
to be expected and benign, so here we capture the warnings and log
messages.
@sphuber sphuber requested a review from unkcpz October 18, 2022 09:16
Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Thanks! The implementation looks all good.
I just wonder, does this also suppresses the warning of deprecation message from aiida-core itself or the dependent packages, which we may expect from the pytest?

@sphuber
Copy link
Contributor Author

sphuber commented Oct 18, 2022

Thanks! The implementation looks all good. I just wonder, does this also suppresses the warning of deprecation message from aiida-core itself or the dependent packages, which we may expect from the pytest?

Not sure what you mean. This suppresses the warning and the logging no matter how this code is invoked. But in this case I think that is warranted, because these warnings and messages are not useful when setting up an automatic temporary profile.

@unkcpz
Copy link
Member

unkcpz commented Oct 18, 2022

I mean if there is some part of the aiida-core code using old deprecated APIs, or the deprecated APIs of other dependent packages, the pytest will find it and report. This PR basically suppresses all warnings, so pytest can not be the source for those warnings.

@sphuber
Copy link
Contributor Author

sphuber commented Oct 18, 2022

I mean if there is some part of the aiida-core code using old deprecated APIs, or the deprecated APIs of other dependent packages, the pytest will find it and report. This PR basically suppresses all warnings, so pytest can not be the source for those warnings.

The messages being suppressed here are not deprecation warnings.

There are two things being suppressed:

  • a UserWarning that is emitted when the .aiida folder is created. Normally this is when you launch AiiDA for the very first time. In this case, it is done by the TemporaryProfileManager because it creates a completely separate AiiDA config folder from scratch. Since that is the intended behavior, we don't need to see the warning each time, so I suppress it here.
  • The TemporaryProfileManager creates a temp profile from scratch, including the temp Postgres database, and then migrates it. This results in a REPORT log message saying it is installing the base database schema. Again, this is not useful to have printed everytime when running the tests with the autogenerated test profile, so I suppress it.

@sphuber sphuber requested a review from unkcpz October 19, 2022 08:59
Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Thanks for explanation @sphuber. It makes all sense. Cheers!

@sphuber sphuber merged commit 16a5637 into aiidateam:main Oct 19, 2022
@sphuber sphuber deleted the fix/suppress-warnings-temporary-profile-fixture branch October 19, 2022 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants