[test][fix] capsys fixture recognizes streams

[M3ssman]

trafficstars

@bertsky I could not completely reproduce what has been mentioned here (https://github.com/OCR-D/core/pull/770):

take the default ocrd_utils/ocrd_logging.conf and modify it in some places, say by defining a logger for qualname=PIL and level=ERROR (which will fail at core/tests/test_decorators.py Line 64 in 9567190 self.assertEqual(logging.getLogger('PIL').getEffectiveLevel(), logging.INFO) ), or by setting the default consoleHandler to sys.stdout instead of stderr.

although I can confirm, that with the current test resources' ocrd_utils/ocrd_logging.conf all three testcases of tests/test_logging_conf.py do fail when default consoleHandler' arg is set to (sys.stdout,) instead of (sys.stderr,). That's caused for the testcases go straight for capsys.readouterr().err which is obviously not present when the consoleHandler is set to sys.stdout. Therefore the new distinction in the tests.

Maybe with the decorator-tests there's some wired interference with the click-handling ...

[M3ssman]

Point taken!

Just copied the ocrd_logging.conf without any change to $USER_HOME and have to learn that suddenly out of nowhere 7 tests go fail; Changed PILs Log level as you mentioned, makes the decorator module also fail. Well, looks like any test that includes validations of log output must be ensured to run in real isolated manner, far more than actually.

Since it concerns just a handful tests, I'll go and review them all.

[M3ssman]

I did some refactoring regarding the logging initialization, to enable injection of logfiles instead of doing this implicitly. But unfortunately, this did not handle all 8 additional failures. Funny fact, that these tests are green when run isolated in VSCode, but not if running the complete testsuite. Especially the test_log and the test_logging modules are tricky to decompose. Anyway IMHO this is a step in the proper direction.

What should be really taken into account: I'd like to see some more conceptual movement from ocrd-stuff to be more like a library, with some notable exceptions (say, the workspace module). I'd like to think in terms of library functionalities with meaningful results or clear exception-handling. Actually there are loggers even on method level (IMHO really overkill) and no dedicated custom exception (like OcrdException), which could be handled by a client application. (Personally I'm pointing to the oai-record-loading things - I'm aware that I'm responsible for this mess, but his must be redone in a proper way. Actually it only logs a warning(!!) if the XML can't be parsed, but the response gets written anyway to disc. But that's a different story.)

@kba : Actual 2 test cases fail from test_workspace, but this module is addressed with https://github.com/OCR-D/core/pull/779 therefore I'd suggest to first apply this one and I will afterwards review this module again before we end in a hard to manage merge situation.

[M3ssman]

Oh no, I've just learned that there are two modules named test_workspace, and the one addressed in https://github.com/OCR-D/core/pull/779 is not the one I've mentioned before. @bertsky @kba Would it be okay to you if I do pytest refactorings of test/cli/test_workspace within the scope of this PR?

[kba]

@kba Would it be okay to you if I do pytest refactorings of test/cli/test_workspace within the scope of this PR?

Sure, thank you

Oh no, I've just learned that there are two modules named test_workspace,

I also sometimes stumbling about those same basenames or that some tests are in subdirectories and some are not. Maybe a flat hierarchy would make it less confusing for developers, e.g. tests/cli/test_workspace.py -> tests/test_cli_workspace.py`?

[M3ssman]

Actually, I was not capable to re-implement some tests with pytest, therefore I left altogether 4 testcases within their prev test setups. Even two of them would fail if a logging configuration is present in USER_HOME, so I marked them to be skipped. I've cut the logging parts off test_add_nonexisting_file and test_init_with_mets_basename_and_not_mets_deprecated . How to deal properly with the OCR-D logging issues - I still don't know. Personally, I decrease logging in my apps as far as possible an/or use a dedicated Logger-Object that encapsulates all logging-related stuff.

[M3ssman]

@kba Renaming the test suite sounds reasonable though. I would even propose to move all test cases that inspect any sort of output to a dedicated test suites for logging and stdout/stderr.

[kba]

The logging has since been changed significantly. But having a logging configuration still interferes with the tests in two places. I'm fixing that and will also add a config option/environment variable OCRD_LOG_BUILTIN_ONLY that globally disables picking up any ocrd_logging.conf. Then I'll cherry-pick your unitttest-to-pytest conversions and close.