Retire patching in __init__ testing

[davidorme]

Description

@vgro and @jacobcook1995

Your models both used the patch routines in testing - I've summarised the changes needed for each set of tests below. If you could have a look and see that it makes sense?

Abiotic

• Created a new fixture that just provides the init data, now used throughout test_abiotic_model.py
• Removed patched calls to AbioticModel()
• Merged two very similar tests
• Removed an unneeded initial definition of the soil_temperature data array from an empty template in AbioticModel._setup(). The variable is populated with real data by a later function.
• Added four variables to AbioticSimpleModel.vars_required_for_init. These aren't strictly used by the model but this is currently the only way to stop the static model checking from freezing up: see the comment below

Abiotic simple

• Created a new fixture that just provides the init data, now used throughout test_abiotic_simple_model.py
• Removed patched calls to AbioticSimpleModel()
• Added seven missing variables to AbioticSimpleModel.vars_required_for_init
• Added two missing variables to AbioticSimpleModel.vars_required_for_update

Hydrology

• Add fixture_hydrology_init_data, now used in test_hydrology_model.py calls
• Removed patched calls to HydrologyModel()
• Added two missing variables to HydrologyModel.vars_required_for_init

Litter

• Add fixture_litter_init_data, now used in fixture_litter_model and test_litter_model.py calls
• Removed patched calls to LitterModel()
• Simplified expected log creation and shared across tests.

Soil model

• Removed unneeded patching in fixture_soil_model

Fixes #1221

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
• Relevant documentation reviewed and updated

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.04%. Comparing base (2c21c86) to head (3dff28b).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1223      +/-   ##
===========================================
- Coverage    95.05%   95.04%   -0.01%     
===========================================
  Files           71       71              
  Lines         7293     7292       -1     
===========================================
- Hits          6932     6931       -1     
  Misses         361      361              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[davidorme]

Oh super. A heisenbug. 👿 Stochastic failures of test_ve_run.

[davidorme]

Oh super. A heisenbug. 👿 Stochastic failures of test_ve_run.

This turned out to be an issue with variable requirements and the model initialisation order sequencing:

There's an issue with the variable config on the Hydrology and Abiotic models. The hydrology model populates these four variables when it initialises (they are in HydrologyModel.vars_populated_by_init)

specific_heat_air, density_air, latent_heat_vapourisation, aerodynamic_resistance_canopy

The abiotic model then has the same variables in AbioticModel.vars_updated

This is a problem because - at the moment - those variables aren't in AbioticModel.vars_required_for_init. So if the model initialisation sequence runs Hydrology first, then the static model checking for the AbioticModel._update method finds the variables and objects that some but not all updated variables are present. I think the problem goes away if we add those variables to AbioticModel.vars_required_for_init because then those variables are specifically excluded from the variables expected for the update step.

This feels like a hack to me - those variables aren't really required for init. I think the issue is that one model populates the variables and another updates them, and I don't think the static model update variable checking anticipated that possibility.

[vgro]

LGTM.
I wonder if the weird behavior could be resolved by moving the initialisation of variables between the models, but I think for now this does the job.

[jacobcook1995]

LGTM (with one minor comment)!