Simplify logic for 'describe#filterByPlatform', as suggested in review

Co-authored-by: Philippe Martin <[email protected]>
redhat-developer · Jul 18, 2023 · 562151e · 562151e
1 parent 974d25c
commit 562151e
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 135 deletions.
diff --git a/pkg/component/describe/describe.go b/pkg/component/describe/describe.go
@@ -82,7 +82,7 @@ func DescribeDevfileComponent(
 		}
 	}
 
-	devControlPlaneData := filterByPlatform(ctx, isApiServerFeatureEnabled, allControlPlaneData, false)
+	devControlPlaneData := filterByPlatform(ctx, isApiServerFeatureEnabled, allControlPlaneData)
 
 	// TODO(feloy) Pass PID with `--pid` flag
 	allFwdPorts, err := stateClient.GetForwardedPorts(ctx)
@@ -96,7 +96,7 @@ func DescribeDevfileComponent(
 			}
 		}
 	}
-	forwardedPorts := filterByPlatform(ctx, isPlatformFeatureEnabled, allFwdPorts, true)
+	forwardedPorts := filterByPlatform(ctx, isPlatformFeatureEnabled, allFwdPorts)
 
 	runningOn, err := GetRunningOn(ctx, componentName, kubeClient, podmanClient)
 	if err != nil {
@@ -229,35 +229,26 @@ func GetRunningOn(ctx context.Context, n string, kubeClient kclient.ClientInterf
 	return runningOn, nil
 }
 
-func filterByPlatform[T platformDependent](ctx context.Context, isFeatEnabled bool, all []T, includeIfFeatDisabled bool) (result []T) {
+func filterByPlatform[T platformDependent](ctx context.Context, isFeatEnabled bool, all []T) (result []T) {
+	if !isFeatEnabled {
+		return nil
+	}
+
 	plt := fcontext.GetPlatform(ctx, "")
 	switch plt {
 	case "":
-		if isFeatEnabled {
-			// Read from all platforms
-			result = all
-		} else if includeIfFeatDisabled {
-			// Limit to cluster only
-			for _, p := range all {
-				if p.GetPlatform() == "" || p.GetPlatform() == commonflags.PlatformCluster {
-					result = append(result, p)
-				}
-			}
-		}
+		// Read from all platforms
+		result = all
 	case commonflags.PlatformCluster:
-		if isFeatEnabled || includeIfFeatDisabled {
-			for _, p := range all {
-				if p.GetPlatform() == "" || p.GetPlatform() == commonflags.PlatformCluster {
-					result = append(result, p)
-				}
+		for _, p := range all {
+			if p.GetPlatform() == "" || p.GetPlatform() == commonflags.PlatformCluster {
+				result = append(result, p)
 			}
 		}
 	case commonflags.PlatformPodman:
-		if isFeatEnabled || includeIfFeatDisabled {
-			for _, p := range all {
-				if p.GetPlatform() == commonflags.PlatformPodman {
-					result = append(result, p)
-				}
+		for _, p := range all {
+			if p.GetPlatform() == commonflags.PlatformPodman {
+				result = append(result, p)
 			}
 		}
 	}

diff --git a/pkg/component/describe/describe_test.go b/pkg/component/describe/describe_test.go
@@ -22,9 +22,8 @@ func (c testType) GetPlatform() string {
 
 func Test_filterByPlatform(t *testing.T) {
 	type args struct {
-		ctx                   context.Context
-		isFeatEnabled         bool
-		includeIfFeatDisabled bool
+		ctx           context.Context
+		isFeatEnabled bool
 	}
 	type testCase struct {
 		name       string
@@ -40,86 +39,26 @@ func Test_filterByPlatform(t *testing.T) {
 	}
 	tests := []testCase{
 		{
-			name: "platform unset in context, isFeatEnabled=true, includeIfFeatDisabled=false",
+			name: "feature disabled",
 			args: args{
-				ctx:                   context.Background(),
-				isFeatEnabled:         true,
-				includeIfFeatDisabled: false,
-			},
-			wantResult: allValues,
-		},
-		{
-			name: "platform unset in context, isFeatEnabled=true, includeIfFeatDisabled=true",
-			args: args{
-				ctx:                   context.Background(),
-				isFeatEnabled:         true,
-				includeIfFeatDisabled: true,
-			},
-			wantResult: allValues,
-		},
-		{
-			name: "platform unset in context, isFeatEnabled=false, includeIfFeatDisabled=true",
-			args: args{
-				ctx:                   context.Background(),
-				isFeatEnabled:         false,
-				includeIfFeatDisabled: true,
-			},
-			wantResult: []testType{
-				{"value without platform", ""},
-				{"value11 (cluster)", "cluster"},
-				{"value12 (cluster)", "cluster"},
-			},
-		},
-		{
-			name: "platform unset in context, isFeatEnabled=false, includeIfFeatDisabled=false",
-			args: args{
-				ctx:                   context.Background(),
-				isFeatEnabled:         false,
-				includeIfFeatDisabled: false,
+				ctx:           context.Background(),
+				isFeatEnabled: false,
 			},
 			wantResult: nil,
 		},
 		{
-			name: "platform set to cluster in context, isFeatEnabled=true, includeIfFeatDisabled=false",
-			args: args{
-				ctx:                   fcontext.WithPlatform(context.Background(), "cluster"),
-				isFeatEnabled:         true,
-				includeIfFeatDisabled: false,
-			},
-			wantResult: []testType{
-				{"value without platform", ""},
-				{"value11 (cluster)", "cluster"},
-				{"value12 (cluster)", "cluster"},
-			},
-		},
-		{
-			name: "platform set to cluster in context, isFeatEnabled=true, includeIfFeatDisabled=true",
-			args: args{
-				ctx:                   fcontext.WithPlatform(context.Background(), "cluster"),
-				isFeatEnabled:         true,
-				includeIfFeatDisabled: true,
-			},
-			wantResult: []testType{
-				{"value without platform", ""},
-				{"value11 (cluster)", "cluster"},
-				{"value12 (cluster)", "cluster"},
-			},
-		},
-		{
-			name: "platform set to cluster in context, isFeatEnabled=false, includeIfFeatDisabled=false",
+			name: "feature enabled and platform unset in context",
 			args: args{
-				ctx:                   fcontext.WithPlatform(context.Background(), "cluster"),
-				isFeatEnabled:         false,
-				includeIfFeatDisabled: false,
+				ctx:           context.Background(),
+				isFeatEnabled: true,
 			},
-			wantResult: nil,
+			wantResult: allValues,
 		},
 		{
-			name: "platform set to cluster in context, isFeatEnabled=false, includeIfFeatDisabled=true",
+			name: "feature enabled and platform set to cluster in context",
 			args: args{
-				ctx:                   fcontext.WithPlatform(context.Background(), "cluster"),
-				isFeatEnabled:         false,
-				includeIfFeatDisabled: true,
+				ctx:           fcontext.WithPlatform(context.Background(), "cluster"),
+				isFeatEnabled: true,
 			},
 			wantResult: []testType{
 				{"value without platform", ""},
@@ -128,44 +67,10 @@ func Test_filterByPlatform(t *testing.T) {
 			},
 		},
 		{
-			name: "platform set to podman in context, isFeatEnabled=true, includeIfFeatDisabled=false",
-			args: args{
-				ctx:                   fcontext.WithPlatform(context.Background(), "podman"),
-				isFeatEnabled:         true,
-				includeIfFeatDisabled: false,
-			},
-			wantResult: []testType{
-				{"value21 (podman)", "podman"},
-				{"value22 (podman)", "podman"},
-			},
-		},
-		{
-			name: "platform set to podman in context, isFeatEnabled=true, includeIfFeatDisabled=true",
-			args: args{
-				ctx:                   fcontext.WithPlatform(context.Background(), "podman"),
-				isFeatEnabled:         true,
-				includeIfFeatDisabled: true,
-			},
-			wantResult: []testType{
-				{"value21 (podman)", "podman"},
-				{"value22 (podman)", "podman"},
-			},
-		},
-		{
-			name: "platform set to podman in context, isFeatEnabled=false, includeIfFeatDisabled=false",
-			args: args{
-				ctx:                   fcontext.WithPlatform(context.Background(), "podman"),
-				isFeatEnabled:         false,
-				includeIfFeatDisabled: false,
-			},
-			wantResult: nil,
-		},
-		{
-			name: "platform set to podman in context, isFeatEnabled=false, includeIfFeatDisabled=true",
+			name: "feature enabled and platform set to podman in context",
 			args: args{
-				ctx:                   fcontext.WithPlatform(context.Background(), "podman"),
-				isFeatEnabled:         false,
-				includeIfFeatDisabled: true,
+				ctx:           fcontext.WithPlatform(context.Background(), "podman"),
+				isFeatEnabled: true,
 			},
 			wantResult: []testType{
 				{"value21 (podman)", "podman"},
@@ -175,7 +80,7 @@ func Test_filterByPlatform(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			gotResult := filterByPlatform(tt.args.ctx, tt.args.isFeatEnabled, allValues, tt.args.includeIfFeatDisabled)
+			gotResult := filterByPlatform(tt.args.ctx, tt.args.isFeatEnabled, allValues)
 			if diff := cmp.Diff(tt.wantResult, gotResult, cmp.AllowUnexported(testType{})); diff != "" {
 				t.Errorf("filterByPlatform() mismatch (-want +got):\n%s", diff)
 			}