python-hospital / hospital

Tools to diagnose Python projects (supervision, monitoring).
Other
40 stars 8 forks source link

hospital-cli should return non-zero exit for failure cases #74

Closed jamesmulcahy closed 9 years ago

jamesmulcahy commented 10 years ago

Fixes #73

jamesmulcahy commented 10 years ago

It's not immediately clear to me that those failures are my fault. They look like environment setup problems?

benoitbryon commented 10 years ago

Hi @jamesmulcahy! Thanks for both the report and pull-request!

If I understood well you expect the following behaviour:

Is that correct?

Can you add assertions/tests about this somewhere in tests/cli.py? So that we make sure we do not break the feature later ;)

benoitbryon commented 10 years ago

Also, we will have to find a way to catch the expected call to sys.exit in existing tests, else the build fails: https://travis-ci.org/python-hospital/hospital/jobs/31271867#L134

benoitbryon commented 10 years ago

we will have to find a way to catch the expected call to sys.exit in existing tests

Or perhaps we could split main() in 2 functions: one that returns result, and one that does the sys.exit() depending on result. Something like that (pseudo-code):

def main():
    result = run_healthchecks()
    if (result.wasSuccessful()):
        sys.exit(0)
    else:
        sys.exit(1)

def run_healthchecks():
    app = HealthCheckProgram()
    return app()

So that we can have tests that focus on run_healthcheck (just like the current tests do). And then we can have additional tests that focus on main (the sys.exit), where we can mock run_healthcheck to return either failure or success.

... but I'm not sure this split is a good idea... So propose any implementation that works, even a naive one. We will be able to improve it later, if we have better ideas.

benoitbryon commented 9 years ago

Added the tests in #82. Merging #82 should automatically merge this one too.