Better handling of file loggers in testing

[davidorme]

Description

This PR:

• Updates virtual_rainforest.core.logger.add_file_logger and virtual_rainforest.core.logger.remove_file_logger to cope with repeated calls to the same function. You cannot add a file handler twice, but removing a non-existent file handler now just exits cleanly.
• A new test suite for that logger functionality.
• Updates the test_vr_run issue from test_vr_run failing all unit tests of logger output to fail #331 to add general exception handling to remove the file handler if things go wrong and also to test that the requested log file is populated.

Fixes #331

Type of change

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist

• Make sure you've run the pre-commit checks: $ pre-commit run -a
• All tests pass: $ poetry run pytest

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[davidorme]

Not quite sure why the Windows builds are failing - something to do with the paths in the logger testing.

The underlying problem seems to be a permissions problem with shutting down a with TemporaryDirectory block being used as an example destination in test_vr_run. Windows seems to not have permission to remove it because another process is using it. My first guess was that the logfile was still being used by the FileHandler so I added an explicit logfile.close() to remove_file_logger, but that hasn't done the trick.

[codecov-commenter]

Codecov Report

Merging #332 (7e5bd8b) into develop (cb19229) will decrease coverage by 0.17%.
Report is 76 commits behind head on develop.
The diff coverage is 97.32%.

@@             Coverage Diff             @@
##           develop     #332      +/-   ##
===========================================
- Coverage    96.43%   96.26%   -0.17%     
===========================================
  Files           48       51       +3     
  Lines         2440     2491      +51     
===========================================
+ Hits          2353     2398      +45     
- Misses          87       93       +6     

Files	Coverage Δ
virtual_rainforest/__init__.py	100.00% <ø> (ø)
virtual_rainforest/core/base_model.py	100.00% <100.00%> (ø)
virtual_rainforest/core/config.py	97.51% <100.00%> (-0.53%)	⬇️
virtual_rainforest/core/constants.py	100.00% <100.00%> (ø)
virtual_rainforest/core/constants_class.py	100.00% <100.00%> (ø)
virtual_rainforest/core/constants_loader.py	100.00% <100.00%> (ø)
virtual_rainforest/core/logger.py	100.00% <100.00%> (ø)
virtual_rainforest/core/schema.py	100.00% <100.00%> (ø)
virtual_rainforest/main.py	100.00% <100.00%> (ø)
...rtual_rainforest/models/abiotic_simple/__init__.py	100.00% <100.00%> (ø)
... and 16 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[jacobcook1995]

LGTM!

[davidorme]

No - apparently I can't read the error log. It is something to do with exiting the with TemporaryDirectory blocks in test_logger.py. There is teardown to ensure the logger is closed when the test exits or fails, but not when the with blocks exit and on Windows, it looks like that open logfile is an issue. So, explicit remove_file_logger() calls at the end of with blocks?