Fix wrong check in function getCompletionTimeFromObject (#237)

* Fix wrong check in function getCompletionTimeFromObject Signed-off-by: John Niang <[email protected]> * Make sure item without completion time be at the end of items
kubesphere-sigs · Oct 29, 2021 · e6f66fb · e6f66fb
1 parent 88e11d9
commit e6f66fb
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 4 deletions.
diff --git a/kubectl-plugin/pipeline/gc.go b/kubectl-plugin/pipeline/gc.go
@@ -2,16 +2,18 @@ package pipeline
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"sort"
+	"time"
+
 	"github.com/kubesphere-sigs/ks/kubectl-plugin/common"
 	"github.com/kubesphere-sigs/ks/kubectl-plugin/pipeline/option"
 	"github.com/kubesphere-sigs/ks/kubectl-plugin/types"
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/client-go/dynamic"
-	"sort"
-	"time"
 )
 
 func newGCCmd(client dynamic.Interface) (cmd *cobra.Command) {
@@ -116,7 +118,8 @@ func ascOrderWithCompletionTime(items []unstructured.Unstructured) {
 			return false
 		}
 		if rightCompletionTime, err = getCompletionTimeFromObject(right.Object); err != nil {
-			return false
+			// make sure that item without completion time be at the end of items
+			return true
 		}
 
 		return leftCompletionTime.Before(rightCompletionTime)
@@ -131,6 +134,9 @@ func getCompletionTimeFromObject(obj map[string]interface{}) (completionTime tim
 	if completionTimeStr, ok, err = unstructured.NestedString(obj, "status", "completionTime"); ok && err == nil {
 		completionTime, err = time.Parse(time.RFC3339, completionTimeStr)
 	}
+	if !ok {
+		err = errors.New("no status.completionTime field found")
+	}
 	return
 }
 

diff --git a/kubectl-plugin/pipeline/gc_test.go b/kubectl-plugin/pipeline/gc_test.go
@@ -1,19 +1,30 @@
 package pipeline
 
 import (
+	"reflect"
+	"testing"
+	"time"
+
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"testing"
 )
 
 func TestDescOrderWithCompletionTime(t *testing.T) {
 	items := []unstructured.Unstructured{{
+		Object: map[string]interface{}{
+			"flag": "3",
+		},
+	}, {
 		Object: map[string]interface{}{
 			"status": map[string]interface{}{
 				"completionTime": "2021-09-28T01:39:13Z",
 			},
 			"flag": "0",
 		},
+	}, {
+		Object: map[string]interface{}{
+			"flag": "4",
+		},
 	}, {
 		Object: map[string]interface{}{
 			"status": map[string]interface{}{
@@ -33,4 +44,83 @@ func TestDescOrderWithCompletionTime(t *testing.T) {
 	ascOrderWithCompletionTime(items)
 	flag, _, _ := unstructured.NestedString(items[0].Object, "flag")
 	assert.Equal(t, "2", flag)
+	// make sure that object without status.completionTime be at the end of the result
+	flag, _, _ = unstructured.NestedString(items[3].Object, "flag")
+	assert.Equal(t, "3", flag)
+	flag, _, _ = unstructured.NestedString(items[4].Object, "flag")
+	assert.Equal(t, "4", flag)
+}
+
+func Test_getCompletionTimeFromObject(t *testing.T) {
+	completionTime := time.Now()
+	completionTimeStr := completionTime.Format(time.RFC3339)
+	expectedCompletionTime, _ := time.Parse(time.RFC3339, completionTimeStr)
+	type args struct {
+		obj map[string]interface{}
+	}
+	tests := []struct {
+		name               string
+		args               args
+		wantCompletionTime time.Time
+		wantErr            bool
+	}{{
+		name: "Nil object",
+		args: args{
+			obj: nil,
+		},
+		wantErr: true,
+	}, {
+		name: "Without completionTime field",
+		args: args{
+			obj: map[string]interface{}{
+				"status": map[string]interface{}{
+					"updateTime": "",
+				},
+			},
+		},
+		wantErr: true,
+	}, {
+		name: "With an empty completionTime",
+		args: args{
+			obj: map[string]interface{}{
+				"status": map[string]interface{}{
+					"completionTime": "",
+				},
+			},
+		},
+		wantErr: true,
+	}, {
+		name: "Has completion time with RFC3339 layout",
+		args: args{
+			obj: map[string]interface{}{
+				"status": map[string]interface{}{
+					"completionTime": completionTime.Format(time.RFC3339),
+				},
+			},
+		},
+		wantCompletionTime: expectedCompletionTime,
+	}, {
+		name: "Has a invalid completion time",
+		args: args{
+			obj: map[string]interface{}{
+				"status": map[string]interface{}{
+					"completionTime": completionTime.Format(time.RFC1123),
+				},
+			},
+		},
+		wantErr: true,
+	},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotCompletionTime, err := getCompletionTimeFromObject(tt.args.obj)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("getCompletionTimeFromObject() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(gotCompletionTime, tt.wantCompletionTime) {
+				t.Errorf("getCompletionTimeFromObject() = %v, want %v", gotCompletionTime, tt.wantCompletionTime)
+			}
+		})
+	}
 }