Merge branch 'main' into zimeg-fix-charm-chevron

Author: zimeg

.circleci/config.yml

@@ -113,7 +113,7 @@ jobs:
         default: "dev-build"
     docker: # run the steps with Docker
       # CircleCI Go images available at: https://hub.docker.com/r/circleci/golang/
-      - image: cimg/go:1.26.0
+      - image: cimg/go:1.26.1
     steps: # steps that comprise the `build` job
       - checkout # check out source code to working directory
       - restore_cache: # restores saved cache if no changes are detected since last run

.claude/CLAUDE.md

@@ -135,6 +135,19 @@ func NewExampleCommand(clients *shared.ClientFactory) *cobra.Command {
 - Mock the `ClientFactory` and its dependencies for testing
 - Always mock file system operations using `afero.Fs` to enable testability
 
+### Test Naming Conventions
+
+Test function names use the format `Test_StructName_FunctionName` for methods on a struct, or `Test_FunctionName` for package-level functions:
+
+```go
+func Test_Client_GetAppStatus(t *testing.T) { ... }  // struct method
+func Test_getKeyLength(t *testing.T) { ... }         // package-level function
+```
+
+### Test Ordering Conventions
+
+Constructor functions (`NewXYZ`) should always be declared first, at the top of the test file. After constructors, test functions should be ordered alphabetically within each file. When a file has logical sections (separated by comments), tests should be alphabetical within each section. Getter and setter functions are grouped together under the base name — ignore the `Get` or `Set` prefix when determining order (e.g. `Test_AppName` and `Test_SetAppName` both sort under `A`). Exceptions to alphabetical ordering can be made when it doesn't work well for readability or logical grouping.
+
 ### Table-Driven Test Conventions
 
 **Preferred: Map pattern** - uses `tc` for test case variable:

.github/MAINTAINERS_GUIDE.md

@@ -383,6 +383,34 @@ The branch name can also be set by changing
 for the `build-lint-test-e2e-test` workflow in the `.circleci/config.yml` file,
 but take care not to merge this change into `main`!
 
+#### Test naming conventions
+
+Test function names should use the format `Test_StructName_FunctionName` for methods
+on a struct, or `Test_FunctionName` for package-level functions. The underscore after
+`Test` separates the Go test prefix from the identifier being tested:
+
+```go
+// Testing a method on a struct
+func Test_Client_GetAppStatus(t *testing.T) { ... }
+
+// Testing a package-level function
+func Test_getKeyLength(t *testing.T) { ... }
+```
+
+#### Test ordering conventions
+
+Constructor functions (`NewXYZ`) should always be declared first, at the top of the
+test file. After constructors, test functions should be ordered alphabetically. When
+a file has logical sections (separated by comments), tests should be alphabetical
+within each section.
+
+Getter and setter functions should be grouped together under the base name. Ignore
+the `Get` or `Set` prefix when determining alphabetical order. For example,
+`Test_AppName` and `Test_SetAppName` are both sorted under `A` for `AppName`.
+
+Exceptions to alphabetical ordering can be made when it doesn't work well for
+readability or logical grouping.
+
 #### Contributing tests
 
 If you'd like to add tests, please review our

.github/workflows/dependencies.yml

@@ -45,7 +45,7 @@ jobs:
     steps:
       - name: Gather credentials
         id: credentials
-        uses: actions/create-github-app-token@29824e69f54612133e76f7eaac726eef6c875baf # v2.2.1
+        uses: actions/create-github-app-token@f8d387b68d61c58ab83c6c016672934102569859 # v3.0.0
         with:
           app-id: ${{ secrets.GH_APP_ID_RELEASER }}
           private-key: ${{ secrets.GH_APP_PRIVATE_KEY_RELEASER }}

.github/workflows/tests.yml

@@ -25,7 +25,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0
         with:
-          go-version: "1.26.0"
+          go-version: "1.26.1"
       - name: Lint
         uses: golangci/golangci-lint-action@1e7e51e771db61008b38414a730f564565cf7c20 # v9.2.0
         with:
@@ -57,7 +57,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0
         with:
-          go-version: "1.26.0"
+          go-version: "1.26.1"
       - name: Report health score
         uses: slackapi/slack-health-score@d58a419f15cdaff97e9aa7f09f95772830ab66f7 # v0.1.1
         with:

go.mod

@@ -1,6 +1,6 @@
 module github.com/slackapi/slack-cli
 
-go 1.26.0
+go 1.26.1
 
 require (
 	charm.land/bubbletea/v2 v2.0.2

internal/iostreams/iostreams_test.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/slackapi/slack-cli/internal/config"
 	"github.com/slackapi/slack-cli/internal/slackdeps"
+	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -34,6 +35,36 @@ func Test_IOSteams_NewIOStreams(t *testing.T) {
 	require.True(t, io.config.DebugEnabled, "iostreams references config")
 }
 
+func Test_IOStreams_ExitCode(t *testing.T) {
+	tests := map[string]struct {
+		setCode  ExitCode
+		expected ExitCode
+	}{
+		"default is ExitOK": {
+			setCode:  ExitOK,
+			expected: ExitOK,
+		},
+		"set to ExitError": {
+			setCode:  ExitError,
+			expected: ExitError,
+		},
+		"set to ExitCancel": {
+			setCode:  ExitCancel,
+			expected: ExitCancel,
+		},
+	}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			fsMock := slackdeps.NewFsMock()
+			osMock := slackdeps.NewOsMock()
+			cfg := config.NewConfig(fsMock, osMock)
+			io := NewIOStreams(cfg, fsMock, osMock)
+			io.SetExitCode(tc.setCode)
+			assert.Equal(t, tc.expected, io.GetExitCode())
+		})
+	}
+}
+
 func Test_IOStreams_IsTTY(t *testing.T) {
 	tests := map[string]struct {
 		fileInfo os.FileInfo
@@ -65,3 +96,15 @@ func Test_IOStreams_IsTTY(t *testing.T) {
 		})
 	}
 }
+
+func Test_IOStreams_SetCmdIO(t *testing.T) {
+	fsMock := slackdeps.NewFsMock()
+	osMock := slackdeps.NewOsMock()
+	cfg := config.NewConfig(fsMock, osMock)
+	io := NewIOStreams(cfg, fsMock, osMock)
+	cmd := &cobra.Command{Use: "test"}
+	io.SetCmdIO(cmd)
+	assert.NotNil(t, cmd.InOrStdin())
+	assert.NotNil(t, cmd.OutOrStdout())
+	assert.NotNil(t, cmd.ErrOrStderr())
+}

internal/iostreams/writer_test.go

@@ -199,6 +199,24 @@ func Test_FilteredWriter(t *testing.T) {
 	}
 }
 
+func Test_IOStreams_WriteErr(t *testing.T) {
+	fsMock := slackdeps.NewFsMock()
+	osMock := slackdeps.NewOsMock()
+	cfg := config.NewConfig(fsMock, osMock)
+	io := NewIOStreams(cfg, fsMock, osMock)
+	w := io.WriteErr()
+	require.NotNil(t, w)
+}
+
+func Test_IOStreams_WriteOut(t *testing.T) {
+	fsMock := slackdeps.NewFsMock()
+	osMock := slackdeps.NewOsMock()
+	cfg := config.NewConfig(fsMock, osMock)
+	io := NewIOStreams(cfg, fsMock, osMock)
+	w := io.WriteOut()
+	require.NotNil(t, w)
+}
+
 func Test_WriteIndent(t *testing.T) {
 	tests := map[string]struct {
 		input    string

internal/runtime/python/python.go

@@ -216,12 +216,13 @@ func installRequirementsTxt(fs afero.Fs, projectDirPath string) (output string,
 }
 
 // installPyProjectToml handles adding slack-cli-hooks to pyproject.toml
-func installPyProjectToml(fs afero.Fs, projectDirPath string) (output string, err error) {
+func installPyProjectToml(ctx context.Context, ios iostreams.IOStreamer, fs afero.Fs, projectDirPath string) (output string, err error) {
 	pyProjectFilePath := filepath.Join(projectDirPath, "pyproject.toml")
 
 	file, err := afero.ReadFile(fs, pyProjectFilePath)
 	if err != nil {
-		return fmt.Sprintf("Error reading pyproject.toml: %s", err), err
+		ios.PrintDebug(ctx, "Warning: could not read pyproject.toml: %s", err)
+		return "Skipped updating pyproject.toml because file cannot be read", nil
 	}
 
 	fileData := string(file)
@@ -235,25 +236,26 @@ func installPyProjectToml(fs afero.Fs, projectDirPath string) (output string, er
 	var config map[string]interface{}
 	err = toml.Unmarshal(file, &config)
 	if err != nil {
-		return fmt.Sprintf("Error parsing pyproject.toml: %s", err), err
+		ios.PrintDebug(ctx, "Warning: could not parse pyproject.toml: %s", err)
+		return "Skipped updating pyproject.toml because invalid TOML", nil
 	}
 
 	// Verify `project` section and `project.dependencies` array exist
 	projectSection, exists := config["project"]
 	if !exists {
-		err := fmt.Errorf("pyproject.toml missing project section")
-		return fmt.Sprintf("Error updating pyproject.toml: %s", err), err
+		ios.PrintDebug(ctx, "Warning: pyproject.toml missing [project] section")
+		return "Skipped updating pyproject.toml because project section missing", nil
 	}
 
 	projectMap, ok := projectSection.(map[string]interface{})
 	if !ok {
-		err := fmt.Errorf("pyproject.toml project section is not a valid format")
-		return fmt.Sprintf("Error updating pyproject.toml: %s", err), err
+		ios.PrintDebug(ctx, "Warning: pyproject.toml [project] section is not a valid format")
+		return "Skipped updating pyproject.toml because project section invalid", nil
 	}
 
 	if _, exists := projectMap["dependencies"]; !exists {
-		err := fmt.Errorf("pyproject.toml missing dependencies array")
-		return fmt.Sprintf("Error updating pyproject.toml: %s", err), err
+		ios.PrintDebug(ctx, "Warning: pyproject.toml missing dependencies array in [project] section")
+		return "Skipped updating pyproject.toml because dependencies missing", nil
 	}
 
 	// Use string manipulation to add the dependency while preserving formatting.
@@ -264,8 +266,8 @@ func installPyProjectToml(fs afero.Fs, projectDirPath string) (output string, er
 	matches := dependenciesRegex.FindStringSubmatch(fileData)
 
 	if len(matches) == 0 {
-		err := fmt.Errorf("pyproject.toml missing dependencies array")
-		return fmt.Sprintf("Error updating pyproject.toml: %s", err), err
+		ios.PrintDebug(ctx, "Warning: pyproject.toml dependencies array could not be located")
+		return "Skipped updating pyproject.toml because dependencies missing", nil
 	}
 
 	prefix := matches[1]  // "...dependencies = ["
@@ -347,16 +349,20 @@ func (p *Python) InstallProjectDependencies(ctx context.Context, projectDirPath
 	// Handle requirements.txt if it exists
 	if hasRequirementsTxt {
 		output, err := installRequirementsTxt(fs, projectDirPath)
-		outputs = append(outputs, output)
+		if output != "" {
+			outputs = append(outputs, output)
+		}
 		if err != nil {
 			errs = append(errs, err)
 		}
 	}
 
 	// Handle pyproject.toml if it exists
 	if hasPyProjectToml {
-		output, err := installPyProjectToml(fs, projectDirPath)
-		outputs = append(outputs, output)
+		output, err := installPyProjectToml(ctx, ios, fs, projectDirPath)
+		if output != "" {
+			outputs = append(outputs, output)
+		}
 		if err != nil {
 			errs = append(errs, err)
 		}

internal/runtime/python/python_test.go

@@ -229,7 +229,13 @@ dependencies = ["pytest==8.3.2"]`,
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
+			ctx := slackcontext.MockContext(t.Context())
 			fs := slackdeps.NewFsMock()
+			os := slackdeps.NewOsMock()
+			os.AddDefaultMocks()
+			cfg := config.NewConfig(fs, os)
+			ios := iostreams.NewIOStreamsMock(cfg, fs, os)
+			ios.AddDefaultMocks()
 			projectDirPath := "/path/to/project"
 
 			// Create pyproject.toml
@@ -240,7 +246,7 @@ dependencies = ["pytest==8.3.2"]`,
 			require.NoError(t, err)
 
 			// Test
-			output, err := installPyProjectToml(fs, projectDirPath)
+			output, err := installPyProjectToml(ctx, ios, fs, projectDirPath)
 
 			// Assertions
 			if tc.expectedError {
@@ -249,7 +255,9 @@ dependencies = ["pytest==8.3.2"]`,
 				require.NoError(t, err)
 			}
 
-			require.Contains(t, output, tc.expectedOutput)
+			if tc.expectedOutput != "" {
+				require.Contains(t, output, tc.expectedOutput)
+			}
 
 			// Verify file content contains expected strings
 			content, err := afero.ReadFile(fs, pyprojectPath)
@@ -527,29 +535,29 @@ dependencies = [
 			expectedOutputs: []string{"Updated requirements.txt"},
 			expectedError:   false,
 		},
-		"Error when pyproject.toml has no dependencies array": {
+		"Warning when pyproject.toml has no dependencies array": {
 			existingFiles: map[string]string{
 				"pyproject.toml": `[project]
 name = "my-app"`,
 			},
-			expectedOutputs: []string{"Error updating pyproject.toml: pyproject.toml missing dependencies array"},
-			expectedError:   true,
+			expectedOutputs: []string{"Skipped updating pyproject.toml because dependencies missing"},
+			expectedError:   false,
 		},
-		"Error when pyproject.toml has no [project] section": {
+		"Warning when pyproject.toml has no [project] section": {
 			existingFiles: map[string]string{
 				"pyproject.toml": `[tool.black]
 line-length = 88`,
 			},
-			expectedOutputs: []string{"Error updating pyproject.toml: pyproject.toml missing project section"},
-			expectedError:   true,
+			expectedOutputs: []string{"Skipped updating pyproject.toml because project section missing"},
+			expectedError:   false,
 		},
-		"Error when pyproject.toml is invalid TOML": {
+		"Warning when pyproject.toml is invalid TOML": {
 			existingFiles: map[string]string{
 				"pyproject.toml": `[project
 name = "broken`,
 			},
-			expectedOutputs: []string{"Error parsing pyproject.toml"},
-			expectedError:   true,
+			expectedOutputs: []string{"Skipped updating pyproject.toml because invalid TOML"},
+			expectedError:   false,
 		},
 	}
 	for name, tc := range tests {