Organise unit tests to use pytest fixtures

SeanBryan51 commented 1 year ago

Unit tests have an excessive amount of repeated test setup code, making unit tests harder to maintain and read. Using pytest fixtures to setup each test would remove needless code repetition and also prevents the possibility of state leaking into each test case.

Organise the tests so that for each function func, we have a test class TestFunc with each method of TestFunc containing a single test (success or failure case).

Use pytest's parametrize() feature for testing different levels of verbosity.

Remove usage of mock_cwd fixture unless it is required for a test to pass.

Fixes #163

codecov[bot] commented 1 year ago

Codecov Report

Merging #172 (107cc86) into main (f083d3e) will decrease coverage by 1.48%. Report is 1 commits behind head on main. The diff coverage is 97.54%.

@@            Coverage Diff             @@
##             main     #172      +/-   ##
==========================================
- Coverage   88.58%   87.10%   -1.48%     
==========================================
  Files          26       25       -1     
  Lines        1603     1613      +10     
==========================================
- Hits         1420     1405      -15     
- Misses        183      208      +25

Files	Coverage Δ
tests/test_benchcab.py	`100.00% <100.00%> (ø)`
tests/test_config.py	`100.00% <100.00%> (ø)`
tests/test_fs.py	`100.00% <100.00%> (ø)`
tests/test_pbs.py	`100.00% <100.00%> (ø)`
tests/test_subprocess.py	`100.00% <100.00%> (ø)`
tests/test_workdir.py	`100.00% <100.00%> (ø)`
tests/conftest.py	`92.98% <91.83%> (-7.02%)`	:arrow_down:
tests/test_comparison.py	`90.90% <90.00%> (-9.10%)`	:arrow_down:
tests/test_repository.py	`97.33% <97.16%> (-2.67%)`	:arrow_down:
tests/test_fluxsite.py	`97.27% <97.12%> (-2.73%)`	:arrow_down:

... and 2 files with indirect coverage changes

SeanBryan51 commented 1 year ago

I have reorganised a few tests files just to see what it looks like. @ccarouge @bschroeter it would be great if I can get your thoughts before I push on further.

SeanBryan51 commented 1 year ago

do we have to do all the tests in the same PR? It might make it very very hard to review. Could we do one module per PR or only a couple at a time?

I think I prefer we do the merge into main as a single PR. But we can treat this branch as 'main' for this issue and create other branches that implement small changes and have those merged into this branch?

ccarouge commented 1 year ago

But we can treat this branch as 'main' for this issue and create other branches that implement small changes and have those merged into this branch?

Yes, that would be better!

SeanBryan51 commented 1 year ago

@ccarouge I've rebased this branch off main so that we have the recently merged changes (e.g. linting with ruff). I've added the changes you requested for using pytest's parametrize() and usage of mock_cwd.

ccarouge commented 1 year ago

@SeanBryan51 It seems you asked my review again on this meta-PR. Is that correct?

SeanBryan51 commented 1 year ago

@ccarouge Yes, just checking I've addressed your comments in: https://github.com/CABLE-LSM/benchcab/pull/172/commits/19eff9e43145c0266ee074bbe1e3b448e5a6415b

CABLE-LSM / benchcab

Organise unit tests to use pytest fixtures #172

Codecov Report