juju command should have tests for return nonzero status code on errors

[Jim Baker]

Currently the ensemble command will return a zero status code for execution errors. (Parsing errors, because of the use of argparse, returns the de facto standard status code of 2.) Presumably we should use status code 1, but regardless it should not be 0.

The mocking in control tests should be changed to reflect this, that is self.setup_exit(1) instead of self.setup_exit(0).

[Clint Byrum (clint-fewbar)]

Raising to Medium. This can be worked around, but it would promote better practices to always return sane error codes.

[Jim Baker (jimbaker)]

This problem seems to be somewhat different than described in the bug report: as verified by actually running them, commands do in fact exit with nonzero status codes (specifically 1) if any errors are raised to the top level of the command. This is done by _handle_exit in juju.control.command.

However, the testing procedure doesn't work here. When tested, reactor.run is mocked out to be a no-op, and returns immediately; then sys.exit, which is almost mocked out, is immediately called next. This occurs before the command has a chance to complete and in particular before the yield on the deferred that is returned by setup_cli_reactor() in CLI tests.

This seems to be a difficult problem to fix so that the desired testing can be done. The run function in command cannot be async - it is what starts the reactor after all. stop should not just exit, the reactor should shut down cleanly. Nor can run do any blocking code and expect that some callback will unblock - the reactor is run in this thread.