fix: return an error if the task fails

[mitchfriedman]

What this PR does / why we need it:
This makes sure that garden doesn't return a 0 exit code if a run test fails.

In our CI, we'd like to run garden run test so that we get the streamed output from the test, instead of garden test where it suppresses the output as it executes.

If the test fails, garden needs to fail with a non-zero status code so our CI run fails as well.

Which issue(s) this PR fixes:

Fixes #1659

[thsig]

Looks good to me, thanks!

[mitchfriedman]

In trying to get the tests to pass, I've removed changes to the task run since they do this work differently AFAICT. I have left this PR as only fixing the test run type.

Also, I had a lot of project changes, so instead I just made a new project and use that in my unit tests.

I think the error handling at the command level might not be correct. Could you take a second look, @thor?

[eysi09]

Whoops, wrong Thor. I was referring to @thsig.

[thsig]

Thanks for the PR, @mitchfriedman! A few things I missed when reviewing this earlier:

• Let's throw a RuntimeError instead of a CommandError.
  • This is for consistency with the "task-based" commands (e.g. TestCommand, DevCommand etc.) which use handleTaskResults.
• We should handle errors the same way in each of the run commands.
  • I'd suggest writing a helper function (could e.g. be called handleActionResult) that checks result.success and returns an error similarly to how you did it here.
  • We could then end the action methods of each of the run commands with

    return handleActionResult(result)
    

    (maybe adding more parameters to the handleActionResult function as needed to generate the appropriate error message text).

[mitchfriedman]

@thsig Thanks for the review and feedback.

I've addressed your feedback about using a RuntimeError instead of CommandError.

I've also added a helper function handleActionResult to allow all the action methods to have a common way to handling errors.

There were 2 issues with this:

• I was unable to use this in the task action type since it runs the task action much differently from the rest - it uses this TaskResult task result type instead of the RunResult that the rest of the actions use. This doesn't have the success value, and instead has it's own error type built into it. I'm not very familiar with typescript and this codebase generally, so I didn't do any major refactors to change this.
• I was unable to create tests for the service/module type since it appears the underlying shell spawn always returns a 0 exit code (perhaps because they actually behave differently i.e. fire and forget?). I'm not sure. I'll leave it to you to decide what to do here.