From fdffdc93be0d9f02071f8ed61d0d60b405818d37 Mon Sep 17 00:00:00 2001
From: phm07 <22707808+phm07@users.noreply.github.com>
Date: Wed, 26 Jun 2024 10:27:06 +0200
Subject: [PATCH 1/4] fix: errors silently ignored when deleting resources

---
 internal/cmd/base/delete.go | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/internal/cmd/base/delete.go b/internal/cmd/base/delete.go
index 5e613ce6..31fa187a 100644
--- a/internal/cmd/base/delete.go
+++ b/internal/cmd/base/delete.go
@@ -85,8 +85,13 @@ func (dc *DeleteCmd) Run(s state.State, cmd *cobra.Command, args []string) error
 			}
 		}
 
+		for _, result := range results {
+			if result.Error != nil {
+				errs = append(errs, result.Error)
+			}
+		}
+
 		if len(actions) > 0 {
-			// TODO: We do not check when an action fail for a specific resource
 			if err := s.WaitForActions(cmd, s, actions...); err != nil {
 				errs = append(errs, err)
 			}

From 971249c8bf352352088d4e5bb7d057969bc967d4 Mon Sep 17 00:00:00 2001
From: phm07 <22707808+phm07@users.noreply.github.com>
Date: Wed, 26 Jun 2024 10:42:51 +0200
Subject: [PATCH 2/4] refactor

---
 internal/cmd/base/delete.go | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/internal/cmd/base/delete.go b/internal/cmd/base/delete.go
index 31fa187a..d6a63d0d 100644
--- a/internal/cmd/base/delete.go
+++ b/internal/cmd/base/delete.go
@@ -88,6 +88,8 @@ func (dc *DeleteCmd) Run(s state.State, cmd *cobra.Command, args []string) error
 		for _, result := range results {
 			if result.Error != nil {
 				errs = append(errs, result.Error)
+			} else {
+				deleted = append(deleted, result.IDOrName)
 			}
 		}
 
@@ -96,12 +98,6 @@ func (dc *DeleteCmd) Run(s state.State, cmd *cobra.Command, args []string) error
 				errs = append(errs, err)
 			}
 		}
-
-		for _, result := range results {
-			if result.Error == nil {
-				deleted = append(deleted, result.IDOrName)
-			}
-		}
 	}
 
 	if len(deleted) == 1 {

From 43812288637099c7008894a224b093c119eded99 Mon Sep 17 00:00:00 2001
From: phm07 <22707808+phm07@users.noreply.github.com>
Date: Wed, 26 Jun 2024 11:01:49 +0200
Subject: [PATCH 3/4] add tests

---
 internal/cmd/base/delete_test.go | 16 ++++++++++++++++
 internal/testutil/cmd.go         |  7 ++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/internal/cmd/base/delete_test.go b/internal/cmd/base/delete_test.go
index 1b2194c4..42fc832f 100644
--- a/internal/cmd/base/delete_test.go
+++ b/internal/cmd/base/delete_test.go
@@ -1,6 +1,7 @@
 package base_test
 
 import (
+	"errors"
 	"sync"
 	"testing"
 
@@ -28,6 +29,11 @@ var fakeDeleteCmd = &base.DeleteCmd{
 		mu.Lock()
 		cmd.Println("Fetching fake resource")
 
+		if idOrName == "fail" {
+			mu.Unlock()
+			return nil, nil, errors.New("this is an error")
+		}
+
 		resource := &fakeResource{
 			ID:   123,
 			Name: "test",
@@ -52,6 +58,16 @@ func TestDelete(t *testing.T) {
 			ExpOut: "Fetching fake resource\nDeleting fake resource\nFetching fake resource\nDeleting fake resource\n" +
 				"Fetching fake resource\nDeleting fake resource\nFake resources 123, 456, 789 deleted\n",
 		},
+		"error": {
+			Args:   []string{"delete", "fail"},
+			ExpOut: "Fetching fake resource\n",
+			ExpErr: "this is an error",
+		},
+		"error multiple": {
+			Args:   []string{"delete", "123", "fail", "789"},
+			ExpOut: "Fetching fake resource\nDeleting fake resource\nFetching fake resource\nFetching fake resource\nDeleting fake resource\nFake resources 123, 789 deleted\n",
+			ExpErr: "this is an error",
+		},
 		"quiet": {
 			Args: []string{"delete", "123", "--quiet"},
 		},
diff --git a/internal/testutil/cmd.go b/internal/testutil/cmd.go
index ad31a459..271d649c 100644
--- a/internal/testutil/cmd.go
+++ b/internal/testutil/cmd.go
@@ -22,6 +22,7 @@ type TestCase struct {
 	ExpOutType    DataType
 	ExpErrOut     string
 	ExpErrOutType DataType
+	ExpErr        string
 }
 
 type DataType string
@@ -72,7 +73,11 @@ func TestCommand(t *testing.T, cmd TestableCommand, cases map[string]TestCase) {
 
 			out, errOut, err := fx.Run(rootCmd, testCase.Args)
 
-			assert.NoError(t, err)
+			if testCase.ExpErr != "" {
+				assert.EqualError(t, err, testCase.ExpErr)
+			} else {
+				assert.NoError(t, err)
+			}
 			testCase.ExpOutType.test(t, testCase.ExpOut, out)
 			testCase.ExpErrOutType.test(t, testCase.ExpErrOut, errOut)
 		})

From 9d7c494eb1cd829e907fa47d89611835df2c7eb5 Mon Sep 17 00:00:00 2001
From: phm07 <22707808+phm07@users.noreply.github.com>
Date: Wed, 26 Jun 2024 11:04:13 +0200
Subject: [PATCH 4/4] add comment back

---
 internal/cmd/base/delete.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/internal/cmd/base/delete.go b/internal/cmd/base/delete.go
index d6a63d0d..d1fb37cd 100644
--- a/internal/cmd/base/delete.go
+++ b/internal/cmd/base/delete.go
@@ -94,6 +94,7 @@ func (dc *DeleteCmd) Run(s state.State, cmd *cobra.Command, args []string) error
 		}
 
 		if len(actions) > 0 {
+			// TODO: We do not check if an action fails for a specific resource
 			if err := s.WaitForActions(cmd, s, actions...); err != nil {
 				errs = append(errs, err)
 			}