[gh cache delete --all] Add --succeed-on-no-caches flag to return exit code 0

[iamazeem]

Fixes #9897

[jtmcg]

Thanks for the contribution, @iamazeem! I've left a few comments throughout the code that I think we'll need to address before we ship this.

Additionally, I'd like to see some tests added to TestDeleteRun to cover these new code paths.

[jtmcg]

Almost there 👍

I still think we're missing unit tests for TestDeleteRun. The tests you've added for NewCmdDelete don't actually test the internals of the command, as they are intercepted by the runF function.

I've gone ahead and opened a PR against your branch to demonstrate what I'm looking for. Notably, one of the tests I wrote is failing, which I think actually caught a case I hadn't considered before trying to write the tests 🥳 (Note: I did not fix the failure, yet, and figured that should be addressed here once that PR merges).

Let me know if you have any questions! And thanks again for all the work you've been putting into the cli lately 🙌

[iamazeem]

@jtmcg: Updated. Ready for review. Thanks!

[williammartin]

Current Behaviour

➜  test-repo git:(feature) gh cache delete --all
X No caches to delete
➜  test-repo git:(feature) echo $?
1

Acceptance Criteria

Given I have no caches
When I run gh cache delete --all --succeed-on-no-caches
Then it exits 0

➜  test-repo git:(feature) ~/workspace/cli/bin/gh cache delete --all --succeed-on-no-caches
✓ No caches to delete
➜  test-repo git:(feature) echo $?
0

When I run gh cache delete <cache-id> --succeed-on-no-caches
Then I get an error that --succeed-on-no-caches can only be used in conjunction with --all

➜  test-repo git:(feature) ~/workspace/cli/bin/gh cache delete --succeed-on-no-caches
--succeed-on-no-caches must be used in conjunction with --all

Usage:  gh cache delete [<cache-id> | <cache-key> | --all] [flags]

Flags:
  -a, --all                          Delete all caches
      --succeed-on-no-caches --all   Return exit code 0 if no caches found. Must be used in conjunction with --all

LGTM thanks!

[williammartin]

@jtmcg please re-review your requested changes.

[jtmcg]

@jtmcg please re-review your requested changes.

Damn, I had reviewed and had forgotten to push "submit review", so I had 2 comments in pending... It's a pretty straight-forward change, then we'll be good to go!

[iamazeem]

@jtmcg: Updated PR for the non-TTY case also. Please review. Thanks!

Tests

$ go clean -testcache && go test ./pkg/cmd/cache/delete/...
ok  	github.com/cli/cli/v2/pkg/cmd/cache/delete	0.005s


# TTY
$ bin/gh cache delete --repo iamazeem/test --succeed-on-no-caches --all
✓ No caches to delete
$ echo $?
0


# non-TTY
$ bin/gh cache delete --repo iamazeem/test --succeed-on-no-caches --all | cat

[jtmcg]

🚀