Skip to content

Commit 3a8c3e5

Browse files
srtaalejsrtaalej
committed
merge: integrate icon assets fallback from #509
Merge ale-icon-assets-fallback into ale-icon-env-var. The resolveIconPath wrapper now delegates to icon.ResolveIconPath for fallback search, gaining assets/ directory support and multiple extensions (.png, .jpg, .jpeg, .gif). Tests updated to use afero.Fs mock instead of os.Chdir.
2 parents 2778c01 + 8df3140 commit 3a8c3e5

6 files changed

Lines changed: 198 additions & 60 deletions

File tree

‎internal/icon/icon.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright 2022-2026 Salesforce, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package icon
16+
17+
import (
18+
"path/filepath"
19+
20+
"github.com/spf13/afero"
21+
)
22+
23+
var SupportedExtensions = []string{".png", ".jpg", ".jpeg", ".gif"}
24+
25+
func ResolveIconPath(fs afero.Fs, manifestIcon string) string {
26+
if manifestIcon != "" {
27+
return manifestIcon
28+
}
29+
for _, dir := range []string{"assets", "."} {
30+
for _, ext := range SupportedExtensions {
31+
candidate := filepath.Join(dir, "icon"+ext)
32+
if _, err := fs.Stat(candidate); err == nil {
33+
return candidate
34+
}
35+
}
36+
}
37+
return ""
38+
}

‎internal/icon/icon_test.go‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright 2022-2026 Salesforce, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package icon
16+
17+
import (
18+
"testing"
19+
20+
"github.com/spf13/afero"
21+
"github.com/stretchr/testify/assert"
22+
"github.com/stretchr/testify/require"
23+
)
24+
25+
func Test_ResolveIconPath(t *testing.T) {
26+
tests := map[string]struct {
27+
manifestIcon string
28+
files []string
29+
expected string
30+
}{
31+
"manifest icon set returns it directly": {
32+
manifestIcon: "custom/my-icon.png",
33+
expected: "custom/my-icon.png",
34+
},
35+
"assets/icon.png found": {
36+
files: []string{"assets/icon.png"},
37+
expected: "assets/icon.png",
38+
},
39+
"assets/icon.jpg found": {
40+
files: []string{"assets/icon.jpg"},
41+
expected: "assets/icon.jpg",
42+
},
43+
"assets/icon.jpeg found": {
44+
files: []string{"assets/icon.jpeg"},
45+
expected: "assets/icon.jpeg",
46+
},
47+
"assets/icon.gif found": {
48+
files: []string{"assets/icon.gif"},
49+
expected: "assets/icon.gif",
50+
},
51+
"png wins over gif in assets": {
52+
files: []string{"assets/icon.png", "assets/icon.gif"},
53+
expected: "assets/icon.png",
54+
},
55+
"jpg wins over gif in assets": {
56+
files: []string{"assets/icon.jpg", "assets/icon.gif"},
57+
expected: "assets/icon.jpg",
58+
},
59+
"root icon.png found when no assets": {
60+
files: []string{"icon.png"},
61+
expected: "icon.png",
62+
},
63+
"root icon.jpg found when no assets": {
64+
files: []string{"icon.jpg"},
65+
expected: "icon.jpg",
66+
},
67+
"assets takes priority over root": {
68+
files: []string{"assets/icon.gif", "icon.png"},
69+
expected: "assets/icon.gif",
70+
},
71+
"no icon files returns empty": {
72+
files: []string{},
73+
expected: "",
74+
},
75+
}
76+
for name, tc := range tests {
77+
t.Run(name, func(t *testing.T) {
78+
fs := afero.NewMemMapFs()
79+
for _, f := range tc.files {
80+
require.NoError(t, afero.WriteFile(fs, f, []byte("img"), 0o644))
81+
}
82+
result := ResolveIconPath(fs, tc.manifestIcon)
83+
assert.Equal(t, tc.expected, result)
84+
})
85+
}
86+
}

‎internal/pkg/apps/install.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ package apps
1717
import (
1818
"context"
1919
"fmt"
20-
"os"
2120
"strings"
2221
"time"
2322

2423
"github.com/opentracing/opentracing-go"
2524
"github.com/slackapi/slack-cli/internal/api"
2625
"github.com/slackapi/slack-cli/internal/config"
2726
"github.com/slackapi/slack-cli/internal/experiment"
27+
"github.com/slackapi/slack-cli/internal/icon"
2828
"github.com/slackapi/slack-cli/internal/pkg/manifest"
2929
"github.com/slackapi/slack-cli/internal/shared"
3030
"github.com/slackapi/slack-cli/internal/shared/types"
@@ -639,28 +639,25 @@ func appendLocalToDisplayName(manifest *types.AppManifest) {
639639
}
640640

641641
// resolveIconPath determines the icon file path using the priority chain:
642-
// SLACK_CLI_APP_ICON_PATH env var > manifest icon field > icon.png in project root
642+
// SLACK_CLI_APP_ICON_PATH env var > manifest icon field > assets/ and root fallback
643643
func resolveIconPath(ctx context.Context, clients *shared.ClientFactory, manifestIcon string) string {
644644
if envIconPath := clients.Config.AppIconPathFlag; envIconPath != "" {
645-
if _, err := os.Stat(envIconPath); !os.IsNotExist(err) {
645+
if _, err := clients.Fs.Stat(envIconPath); err == nil {
646646
return envIconPath
647647
}
648648
clients.IO.PrintDebug(ctx, "SLACK_CLI_APP_ICON_PATH file not found: %s", envIconPath)
649649
_, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Warning: icon path from SLACK_CLI_APP_ICON_PATH not found: %s", envIconPath)))
650650
return ""
651651
}
652652
if manifestIcon != "" {
653-
if _, err := os.Stat(manifestIcon); !os.IsNotExist(err) {
653+
if _, err := clients.Fs.Stat(manifestIcon); err == nil {
654654
return manifestIcon
655655
}
656656
clients.IO.PrintDebug(ctx, "manifest icon file not found: %s", manifestIcon)
657657
_, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Warning: icon path from manifest not found: %s", manifestIcon)))
658658
return ""
659659
}
660-
if _, err := os.Stat("icon.png"); !os.IsNotExist(err) {
661-
return "icon.png"
662-
}
663-
return ""
660+
return icon.ResolveIconPath(clients.Fs, "")
664661
}
665662

666663
// updateIcon will upload the new icon to the Slack API

‎internal/pkg/apps/install_test.go‎

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ import (
1818
"bytes"
1919
"context"
2020
"fmt"
21-
"os"
22-
"path/filepath"
2321
"testing"
2422

2523
"github.com/slackapi/slack-cli/internal/api"
@@ -31,6 +29,7 @@ import (
3129
"github.com/slackapi/slack-cli/internal/shared/types"
3230
"github.com/slackapi/slack-cli/internal/slackcontext"
3331
"github.com/slackapi/slack-cli/internal/slackerror"
32+
"github.com/spf13/afero"
3433
"github.com/stretchr/testify/assert"
3534
"github.com/stretchr/testify/mock"
3635
"github.com/stretchr/testify/require"
@@ -1734,78 +1733,72 @@ func Test_resolveIconPath(t *testing.T) {
17341733
tests := map[string]struct {
17351734
envIconPath string
17361735
manifestIcon string
1737-
setupFiles func(t *testing.T, dir string)
1736+
setupFiles func(t *testing.T, fs afero.Fs)
17381737
expected string
17391738
}{
17401739
"env var takes priority over manifest icon": {
17411740
envIconPath: "env-icon.png",
17421741
manifestIcon: "manifest-icon.png",
1743-
setupFiles: func(t *testing.T, dir string) {
1744-
require.NoError(t, os.WriteFile(filepath.Join(dir, "env-icon.png"), []byte("img"), 0o644))
1745-
require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest-icon.png"), []byte("img"), 0o644))
1742+
setupFiles: func(t *testing.T, fs afero.Fs) {
1743+
require.NoError(t, afero.WriteFile(fs, "env-icon.png", []byte("img"), 0o644))
1744+
require.NoError(t, afero.WriteFile(fs, "manifest-icon.png", []byte("img"), 0o644))
17461745
},
17471746
expected: "env-icon.png",
17481747
},
1749-
"env var takes priority over icon.png fallback": {
1748+
"env var takes priority over fallback": {
17501749
envIconPath: "env-icon.png",
1751-
setupFiles: func(t *testing.T, dir string) {
1752-
require.NoError(t, os.WriteFile(filepath.Join(dir, "env-icon.png"), []byte("img"), 0o644))
1753-
require.NoError(t, os.WriteFile(filepath.Join(dir, "icon.png"), []byte("img"), 0o644))
1750+
setupFiles: func(t *testing.T, fs afero.Fs) {
1751+
require.NoError(t, afero.WriteFile(fs, "env-icon.png", []byte("img"), 0o644))
1752+
require.NoError(t, afero.WriteFile(fs, "assets/icon.png", []byte("img"), 0o644))
17541753
},
17551754
expected: "env-icon.png",
17561755
},
17571756
"manifest icon used when no env var": {
17581757
manifestIcon: "manifest-icon.png",
1759-
setupFiles: func(t *testing.T, dir string) {
1760-
require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest-icon.png"), []byte("img"), 0o644))
1758+
setupFiles: func(t *testing.T, fs afero.Fs) {
1759+
require.NoError(t, afero.WriteFile(fs, "manifest-icon.png", []byte("img"), 0o644))
17611760
},
17621761
expected: "manifest-icon.png",
17631762
},
1764-
"falls back to icon.png when no env var or manifest": {
1765-
setupFiles: func(t *testing.T, dir string) {
1766-
require.NoError(t, os.WriteFile(filepath.Join(dir, "icon.png"), []byte("img"), 0o644))
1763+
"falls back to assets/icon.png when no env var or manifest": {
1764+
setupFiles: func(t *testing.T, fs afero.Fs) {
1765+
require.NoError(t, afero.WriteFile(fs, "assets/icon.png", []byte("img"), 0o644))
1766+
},
1767+
expected: "assets/icon.png",
1768+
},
1769+
"falls back to icon.png in root when no assets": {
1770+
setupFiles: func(t *testing.T, fs afero.Fs) {
1771+
require.NoError(t, afero.WriteFile(fs, "icon.png", []byte("img"), 0o644))
17671772
},
17681773
expected: "icon.png",
17691774
},
17701775
"returns empty when no icon found": {
1771-
setupFiles: func(t *testing.T, dir string) {},
1776+
setupFiles: func(t *testing.T, fs afero.Fs) {},
17721777
expected: "",
17731778
},
17741779
"env var file not found returns empty": {
17751780
envIconPath: "missing-icon.png",
17761781
manifestIcon: "manifest-icon.png",
1777-
setupFiles: func(t *testing.T, dir string) {
1778-
require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest-icon.png"), []byte("img"), 0o644))
1782+
setupFiles: func(t *testing.T, fs afero.Fs) {
1783+
require.NoError(t, afero.WriteFile(fs, "manifest-icon.png", []byte("img"), 0o644))
17791784
},
17801785
expected: "",
17811786
},
17821787
"manifest icon file not found returns empty with warning": {
17831788
manifestIcon: "missing-manifest-icon.png",
1784-
setupFiles: func(t *testing.T, dir string) {
1785-
require.NoError(t, os.WriteFile(filepath.Join(dir, "icon.png"), []byte("img"), 0o644))
1789+
setupFiles: func(t *testing.T, fs afero.Fs) {
1790+
require.NoError(t, afero.WriteFile(fs, "assets/icon.png", []byte("img"), 0o644))
17861791
},
17871792
expected: "",
17881793
},
1789-
"manifest icon file not found and no icon.png returns empty": {
1790-
manifestIcon: "missing-manifest-icon.png",
1791-
setupFiles: func(t *testing.T, dir string) {},
1792-
expected: "",
1793-
},
17941794
}
17951795
for name, tc := range tests {
17961796
t.Run(name, func(t *testing.T) {
1797-
dir := t.TempDir()
1798-
tc.setupFiles(t, dir)
1799-
1800-
origDir, err := os.Getwd()
1801-
require.NoError(t, err)
1802-
require.NoError(t, os.Chdir(dir))
1803-
defer func() { require.NoError(t, os.Chdir(origDir)) }()
1804-
18051797
ctx := slackcontext.MockContext(t.Context())
18061798
clientsMock := shared.NewClientsMock()
18071799
clientsMock.AddDefaultMocks()
18081800
clientsMock.Config.AppIconPathFlag = tc.envIconPath
1801+
tc.setupFiles(t, clientsMock.Fs)
18091802
output := &bytes.Buffer{}
18101803
clientsMock.IO.Stdout.SetOutput(output)
18111804
clients := shared.NewClientFactory(clientsMock.MockClientFactory())

‎internal/shared/types/slack_yaml.go‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ import (
1818
"os"
1919
"path/filepath"
2020

21+
"github.com/slackapi/slack-cli/internal/icon"
2122
"github.com/slackapi/slack-cli/internal/slackerror"
23+
"github.com/spf13/afero"
2224
)
2325

2426
type SlackYaml struct {
@@ -28,28 +30,24 @@ type SlackYaml struct {
2830
}
2931

3032
// hasValidIconPath returns false if icon path is provided but is not valid and true otherwise
31-
func (sy *SlackYaml) hasValidIconPath() bool {
32-
// verify icon path is valid if exists
33-
var wd, err = os.Getwd()
34-
if err == nil {
35-
if sy.Icon == "" { // icon was not provided. Let's check if the default one exists
36-
var defaultIconPath = "assets/icon.png"
37-
if _, err := os.Stat(filepath.Join(wd, defaultIconPath)); !os.IsNotExist(err) {
38-
sy.Icon = defaultIconPath
39-
}
40-
} else {
41-
if _, err := os.Stat(filepath.Join(wd, sy.Icon)); os.IsNotExist(err) {
42-
return false
43-
}
33+
func (sy *SlackYaml) hasValidIconPath(fs afero.Fs) bool {
34+
wd, err := os.Getwd()
35+
if err != nil {
36+
return true
37+
}
38+
if sy.Icon == "" {
39+
sy.Icon = icon.ResolveIconPath(afero.NewBasePathFs(fs, wd), "")
40+
} else {
41+
if _, err := fs.Stat(filepath.Join(wd, sy.Icon)); os.IsNotExist(err) {
42+
return false
4443
}
4544
}
46-
4745
return true
4846
}
4947

5048
// Verify checks that the app manifest meets some basic requirements
51-
func (sy *SlackYaml) Verify() error {
52-
if !sy.hasValidIconPath() {
49+
func (sy *SlackYaml) Verify(fs afero.Fs) error {
50+
if !sy.hasValidIconPath(fs) {
5351
return slackerror.New("Please specify a valid icon path in app manifest")
5452
}
5553
return nil

0 commit comments

Comments
 (0)