Conversation
Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
09117bd to
d7a5e4c
Compare
There was a problem hiding this comment.
Code Review Summary
I've reviewed the added code in this PR and found no issues. The implementation is clean and correct.
Changes Reviewed
✅ cmd/root/root.go - New setErrorHandlingRecursive function properly wraps error handling for all subcommands and their children. The recursive implementation correctly applies error processing to the entire command tree.
✅ e2e/binary/shellout.go - Test utility functions are well-structured with proper error handling. The ExecWithContextInDir function correctly inherits environment variables as intended for test scenarios.
✅ e2e/binary/binary_test.go - Tests cover the important execution modes (CLI plugin, cagent, docker-agent) and validate both help output and error handling.
✅ Taskfile.yml - Build and test tasks are properly configured with the new test-binary task depending on deploy-local to ensure binaries are built before testing.
✅ .github/workflows/ci.yml - Workflow correctly adds task test-binary to the test step.
The PR successfully addresses the stated goal of fixing error handling for subcommands when running as a docker cli plugin.
Also fixes error handling not executing on subcommands when running docker cli plugin
Adds only a few secs to the CI tests, since it's already all compiled for
go test