search

NOAA-GFDL / fre-cli

Python-based command line interface for FRE (FMS Runtime Environment) to compile and run FMS-based models and post-process their output.
GNU Lesser General Public License v3.0
3 stars 7 forks source link

Initial pytest efforts for cli calls #120

Closed ilaflott closed 2 months ago

ilaflott commented 2 months ago

This PR adds basic pytest functionality covering CLI calls fre, fre --help and fre optionDNE, i.e. it makes sure they all give the correct return code.

ilaflott commented 2 months ago

this is not ready for merging. the pytest call in the CI pipeline is not passing.

local tests are fine though via manual clone + install.

ilaflott commented 2 months ago

Woohoo! Thanks Ian, just what we need. I sort liked the separate "Pytest all the code" and "Pylint all the code" sections, but it's just cosmetic.

I did too! It's also totally redundant with the "test" field in meta.yaml, and it's easier to call the tools there anyways. no redundant pip-installs etc. needed- everything being used is in the python environment properly.

I almost have this exactly where i want it!

ilaflott commented 2 months ago

OK this is ready and I am merging after this comment.

Biggest "got ya" here is easily going to be the pylint call here. This is needed because pylint throws a non-zero exit code for the presence of ANY complaint at all. very annoying, but can be guarded against with a || echo "FOO" after. the || means "execute only if prev command throws non-zero exit code", and the echo "FOO" is for running a command that trivially throws an exit code of 0, letting us read pylint complaints without nuking the rest of the workflow.

I would prefer pylint and pytest steps be separate workflow steps, or even better yet, separate workflow files/jobs, but given that only so much info can be passed between steps/jobs/flows etc, it currently feels simpler to bootstrap this to the conda build step by putting the pylint and pytest calls in the meta.yaml.

test directories moved around- tests/ was moved to fre/tests as this dir contains scripts that test fre, fre --help, fre optionDNE style calls. fre/app/tests was moved back to fre/app/generate_time_averages/tests, as it only contains tests for generate_time_averages.

i.e., the tests for fre <command> will always be in fre/<command>/tests. In the case of fre app <subcommand>, they will be in fre/app/<subcommand>/tests. this is in line with general TDD practices in python.