search

aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
431 stars 187 forks source link

fix skipped test or remove? #4292

Closed ltalirz closed 3 months ago

ltalirz commented 4 years ago

Hi @CasperWA , while checking skipped tests, I came across this: https://github.com/aiidateam/aiida-core/blob/46af33022f49234307534c40089c0865103e4665/tests/cmdline/commands/test_import.py#L148-L162

Is this a "note to self" to change the test in some way? I suggest you modify the test or you simply delete it.

CasperWA commented 4 years ago

The main issue here is that I am having severe trouble retrieving the logged overview about the conditions with which the import command was called. For some reason it is not outputted to the CliRunner's test output, and pytest is not finding it using the caplog (or any other similar) fixture.

For now, it seems to me this cannot be tested. At least not by looking at the simple summary output from the import function. Instead, one would have to make a more thorough test that these options are respected, similar to the more internal testing of the import function, something that I would like to avoid here, to be honest.

I guess we can live with this not being tested, as the underlying functionality is still tested elsewhere, and only the connection between the verdi option and the import function is not tested here?

ltalirz commented 4 years ago

If the actual content of the archive created with different comment modes is already tested elsewhere, then I agree that this test is nice-to-have but not super critical.

Do you have an idea why the overview table is not in the output captured by click? It probably has to do with the fact that it is written using the import logger here:

https://github.com/aiidateam/aiida-core/blob/855ae82e42b8a50e6c507fe9083187a22fe2cfea/aiida/tools/importexport/dbimport/utils.py#L265

Perhaps the logger realizes it's not used in interactive mode, or the log level is set too low for some reason.

P.S. I will continue working on https://github.com/aiidateam/aiida-core/pull/3896 now, which includes an aiida-wide log level configuration; perhaps in the process we will learn what is the problem here as well.

CasperWA commented 4 years ago

Do you have an idea why the overview table is not in the output captured by click? It probably has to do with the fact that it is written using the import logger here:

https://github.com/aiidateam/aiida-core/blob/855ae82e42b8a50e6c507fe9083187a22fe2cfea/aiida/tools/importexport/dbimport/utils.py#L265

Perhaps the logger realizes it's not used in interactive mode, or the log level is set too low for some reason.

I think it's more likely that the logging output is not captured by click's special testing capture wrapper. The log level should be fine, it's using 23 (between INFO and WARNING).

P.S. I will continue working on #3896 now, which includes an aiida-wide log level configuration; perhaps in the process we will learn what is the problem here as well.

:+1: I'll leave this be for a bit then and see what you come up with.

ltalirz commented 4 years ago

The suppression of output in tests was caused by the automatic filter for tests configured on AiiDA's console handler. https://github.com/aiidateam/aiida-core/blob/56ffc59f8296c589edd5da06817e23eb54f01fbd/aiida/common/log.py#L99-L104

I guess your logger is using that handler - in order to prevent it, simply set the filters of the logger to an empty list (or use the click handler introduced in #3896).