Add acceptance tests for SAMOS CLIs where nothing should be returned

[brhooper]

Various paths now exist within the estimate_samos* CLIs where nothing should be returned due to missing inputs. This PR adds acceptance tests for those situations.

Depends on: #2218

Testing:

• Ran tests and they passed OK
• Added new tests for the new feature(s)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.10%. Comparing base (84a8944) to head (f5779a9).
⚠️ Report is 136 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2219      +/-   ##
==========================================
- Coverage   98.39%   95.10%   -3.30%     
==========================================
  Files         124      147      +23     
  Lines       12212    14867    +2655     
==========================================
+ Hits        12016    14139    +2123     
- Misses        196      728     +532     

☔ 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.

[maxwhitemet]

I am happy the tests cover the required tests cases, but I have made some very minor suggestions to enhance clarity.

[brhooper]

Thanks for your review @maxwhitemet. I've modified most of the doc-strings as you've suggested to more explicitly say 'returns None' where appropriate.

Some of your comments suggest explaining why we want certain outcomes in the test. I've not added these as I believe they should be described in the plugins/CLIs themselves rather than associated tests.

[maxwhitemet]

Happy with the changes made. Approved 👍

[bayliffe]

There is a problem with all of these tests, in that they don't test anything. The run_cli call will assert as None for all of your valid tests that don't return None as well (have a play). The call to run_cli executes the CLI call which leads to a file being written, but you don't get anything else back in most (all?) cases.

improver/improver/cli/__init__.py

Line 582 in b25dd40

result = exec_cmd(

Trying to follow this through is not very fun, so it's easier just to test working tests that don't return None and see that you always get None as a result of the run_cli call.

You will need to check that there is no output as a result of the tests you've added, something like:
assert not output_path.exists()

@maxwhitemet have another look at this and play around so you get a better understanding, and think about how you might test this in future.

[brhooper]

There is a problem with all of these tests, in that they don't test anything. The run_cli call will assert as None for all of your valid tests that don't return None as well (have a play). The call to run_cli executes the CLI call which leads to a file being written, but you don't get anything else back in most (all?) cases.

improver/improver/cli/__init__.py

Line 582 in b25dd40

result = exec_cmd(

Trying to follow this through is not very fun, so it's easier just to test working tests that don't return None and see that you always get None as a result of the run_cli call.

You will need to check that there is no output as a result of the tests you've added, something like: assert not output_path.exists()

@maxwhitemet have another look at this and play around so you get a better understanding, and think about how you might test this in future.

Thanks @bayliffe. I've changed all instances of assert run_cli(args) is None to assert not output_path.exists(). The same issue existed in one test in improver_tests/acceptance/test_estimate_emos_coefficients_from_table.py, so I've fixed that too.

[bayliffe]

Thanks Ben.