fix: add deprecation comment and unit tests for updateIcon routing

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Author: srtaalej

internal/pkg/apps/install.go

@@ -659,6 +659,7 @@ func updateIcon(ctx context.Context, clients *shared.ClientFactory, iconPath, ap
 	if clients.Config.WithExperimentOn(experiment.SetIcon) {
 		_, err = clients.API().IconSet(ctx, clients.Fs, token, appID, iconPath)
 	} else if isHosted {
+		// DEPRECATED: Prefer IconSet once the SetIcon experiment concludes
 		_, err = clients.API().Icon(ctx, clients.Fs, token, appID, iconPath)
 	} else {
 		return nil

internal/pkg/apps/install_test.go

@@ -16,12 +16,15 @@ package apps
 
 import (
 	"bytes"
+	"context"
+	"fmt"
 	"testing"
 
 	"github.com/slackapi/slack-cli/internal/api"
 	"github.com/slackapi/slack-cli/internal/app"
 	"github.com/slackapi/slack-cli/internal/cache"
 	"github.com/slackapi/slack-cli/internal/config"
+	"github.com/slackapi/slack-cli/internal/experiment"
 	"github.com/slackapi/slack-cli/internal/shared"
 	"github.com/slackapi/slack-cli/internal/shared/types"
 	"github.com/slackapi/slack-cli/internal/slackcontext"
@@ -1724,3 +1727,87 @@ func TestContinueDespiteWarning(t *testing.T) {
 		})
 	}
 }
+
+func Test_updateIcon(t *testing.T) {
+	tests := map[string]struct {
+		isHosted      bool
+		experimentOn  bool
+		expectIconSet bool
+		expectIcon    bool
+		expectSkip    bool
+		mockError     error
+		expectedError bool
+	}{
+		"experiment on + hosted app uses IconSet": {
+			isHosted:      true,
+			experimentOn:  true,
+			expectIconSet: true,
+		},
+		"experiment on + non-hosted app uses IconSet": {
+			isHosted:      false,
+			experimentOn:  true,
+			expectIconSet: true,
+		},
+		"experiment off + hosted app uses Icon": {
+			isHosted:     true,
+			experimentOn: false,
+			expectIcon:   true,
+		},
+		"experiment off + non-hosted app skips upload": {
+			isHosted:     false,
+			experimentOn: false,
+			expectSkip:   true,
+		},
+		"returns error from IconSet": {
+			isHosted:      false,
+			experimentOn:  true,
+			expectIconSet: true,
+			mockError:     fmt.Errorf("api error"),
+			expectedError: true,
+		},
+		"returns error from Icon": {
+			isHosted:      true,
+			experimentOn:  false,
+			expectIcon:    true,
+			mockError:     fmt.Errorf("api error"),
+			expectedError: true,
+		},
+	}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			ctx := slackcontext.MockContext(t.Context())
+			clientsMock := shared.NewClientsMock()
+			clientsMock.AddDefaultMocks()
+
+			if tc.experimentOn {
+				clientsMock.Config.ExperimentsFlag = []string{string(experiment.SetIcon)}
+				clientsMock.Config.LoadExperiments(ctx, func(_ context.Context, _ string, _ ...interface{}) {})
+			}
+
+			clientsMock.API.On("IconSet", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+				Return(api.IconResult{}, tc.mockError)
+			clientsMock.API.On("Icon", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+				Return(api.IconResult{}, tc.mockError)
+
+			clients := shared.NewClientFactory(clientsMock.MockClientFactory())
+			err := updateIcon(ctx, clients, "icon.png", "A001", "xoxe-token", tc.isHosted)
+
+			if tc.expectedError {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+
+			if tc.expectIconSet {
+				clientsMock.API.AssertCalled(t, "IconSet", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+				clientsMock.API.AssertNotCalled(t, "Icon")
+			} else if tc.expectIcon {
+				clientsMock.API.AssertCalled(t, "Icon", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+				clientsMock.API.AssertNotCalled(t, "IconSet")
+			} else if tc.expectSkip {
+				clientsMock.API.AssertNotCalled(t, "Icon")
+				clientsMock.API.AssertNotCalled(t, "IconSet")
+			}
+		})
+	}
+}