Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions cmd/platform/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,14 @@ var runAppSelectPromptFunc = prompts.AppSelectPrompt

func NewRunCommand(clients *shared.ClientFactory) *cobra.Command {
cmd := &cobra.Command{
Use: "run",
Use: "run [app-path]",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📚 note: I'm not so excited about this placeholder but it's clear to me...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd clarify that it's a filepath (src/app.js) instead of a path (src/). Otherwise, I think it's clear!

You could use run [file-path] if run [app-file-path] reads strange to you.

Suggested change
Use: "run [app-path]",
Use: "run [app-file-path]",

Aliases: []string{"dev", "start-dev"}, // Aliases a few proposed alternative names
Short: "Start a local server to develop and run the app locally",
Long: `Start a local server to develop and run the app locally while watching for file changes`,
Args: cobra.MaximumNArgs(1),
Example: style.ExampleCommandsf([]style.ExampleCommand{
{Command: "platform run", Meaning: "Start a local development server"},
{Command: "platform run --activity-level debug", Meaning: "Run a local development server with debug activity"},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗣️ note: IIRC this activity level flag isn't used often so we might encourage examples that outline another common use case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👾 ramble: I'm now curious if this flag updates a LOG_LEVEL environment variable for run command?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, we can probably remove it as an example since it's Deno/ROSI specific as well.

thought: In the future, we will probably introduce a --log-level flag that the CLI will use and pass into Bolt Frameworks. The same log level should be passed into the Deno SDK and could be inherited by --activity-level for Deno apps.

The end result would be a universal log-level flag that works for all frameworks and ROSI.

{Command: "platform run ./src/app.py", Meaning: "Run a local development server with a custom app entry point"},
{Command: "platform run --cleanup", Meaning: "Run a local development server with cleanup"},
}),
PreRunE: func(cmd *cobra.Command, args []string) error {
Expand Down Expand Up @@ -96,6 +97,16 @@ func RunRunCommand(clients *shared.ClientFactory, cmd *cobra.Command, args []str
}
ctx := cmd.Context()

var appPath string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Could I suggest appFilePath? To me appPath implies a path (directory path) while appFilePath implies a file path (path to a specific file).

if len(args) > 0 {
appPath = args[0]
if _, err := clients.Fs.Stat(appPath); err != nil {
return slackerror.New(slackerror.ErrNotFound).
WithMessage("The app path %q could not be found", appPath).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
WithMessage("The app path %q could not be found", appPath).
WithMessage("The app file path %q could not be found", appPath).

WithRemediation("Check that the file exists and the path is correct")
}
}
Comment on lines +100 to +108
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var appPath string
if len(args) > 0 {
appPath = args[0]
if _, err := clients.Fs.Stat(appPath); err != nil {
return slackerror.New(slackerror.ErrNotFound).
WithMessage("The app path %q could not be found", appPath).
WithRemediation("Check that the file exists and the path is correct")
}
}

🪓 note: I'm curious of removing this in favor of leaving errors to the hook implementations. Removing this might let for strange argument parsing as perhaps:

$ slack run localhost

👾 ramble: For now it seems to make a better experience for the intended filepath arguments I think-

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: Interesting idea to leave this to the framework. I think for now, we should leave this in the CLI. We can loosen it later if there is a feature request, but this starts with ensuring an error-free experience. If developers are running a build (e.g. TypeScript: dist/index.js) then they'll need to start their watch server or build a dist before starting the Slack CLI.


// Get the workspace from the flag or prompt
selection, err := runAppSelectPromptFunc(ctx, clients, prompts.ShowLocalOnly, prompts.ShowAllApps)
if err != nil {
Expand Down Expand Up @@ -136,6 +147,7 @@ func RunRunCommand(clients *shared.ClientFactory, cmd *cobra.Command, args []str
Activity: !runFlags.noActivity,
ActivityLevel: runFlags.activityLevel,
App: selection.App,
AppPath: appPath,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: You know my nit here, lol.

Suggested change
AppPath: appPath,
AppFilePath: appFilePath,

Auth: selection.Auth,
Cleanup: runFlags.cleanup,
ShowTriggers: triggers.ShowTriggers(clients, runFlags.hideTriggers),
Expand Down
34 changes: 34 additions & 0 deletions cmd/platform/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/slackapi/slack-cli/internal/slackerror"
"github.com/slackapi/slack-cli/internal/style"
"github.com/slackapi/slack-cli/test/testutil"
"github.com/spf13/afero"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand All @@ -54,6 +55,7 @@ func (m *RunPkgMock) Run(ctx context.Context, clients *shared.ClientFactory, run

func TestRunCommand_Flags(t *testing.T) {
tests := map[string]struct {
setup func(cm *shared.ClientsMock)
cmdArgs []string
appFlag string
tokenFlag string
Expand Down Expand Up @@ -186,6 +188,35 @@ func TestRunCommand_Flags(t *testing.T) {
},
expectedErr: slackerror.New(slackerror.ErrProcessInterrupted),
},
"Positional arg sets AppPath": {
setup: func(cm *shared.ClientsMock) {
_ = afero.WriteFile(cm.Fs, "./src/app.py", []byte(""), 0644)
},
cmdArgs: []string{"./src/app.py"},
selectedAppAuth: prompts.SelectedApp{
App: types.NewApp(),
Auth: types.SlackAuth{},
},
expectedRunArgs: platform.RunArgs{
Activity: true,
ActivityLevel: "info",
App: types.NewApp(),
AppPath: "./src/app.py",
Auth: types.SlackAuth{},
Cleanup: false,
ShowTriggers: true,
},
},
"Error if app path does not exist": {
cmdArgs: []string{"./nonexistent/app.py"},
selectedAppAuth: prompts.SelectedApp{
App: types.NewApp(),
Auth: types.SlackAuth{},
},
expectedErr: slackerror.New(slackerror.ErrNotFound).
WithMessage("The app path %q could not be found", "./nonexistent/app.py").
WithRemediation("Check that the file exists and the path is correct"),
},
"Error if no apps are available when using a remote manifest source": {
selectedAppErr: slackerror.New(slackerror.ErrMissingOptions),
expectedErr: slackerror.New(slackerror.ErrAppNotFound).
Expand Down Expand Up @@ -214,6 +245,9 @@ func TestRunCommand_Flags(t *testing.T) {
clients.Config.AppFlag = tc.appFlag
clients.Config.TokenFlag = tc.tokenFlag
})
if tc.setup != nil {
tc.setup(clientsMock)
}

appSelectMock := prompts.NewAppSelectMock()
appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowLocalOnly, prompts.ShowAllApps).Return(tc.selectedAppAuth, tc.selectedAppErr)
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/hooks/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ The application's app-level token and bot access token will be provided as envir

All Bolt SDKs leverage this `start` hook operating mode.

A custom start path can be set with the `SLACK_CLI_CUSTOM_FILE_PATH` variable.
A custom start path can be provided as a positional argument to the `run` command (e.g., `slack run ./src/app.py`), which sets both the `SLACK_APP_PATH` and `SLACK_CLI_CUSTOM_FILE_PATH` environment variables for the hook process.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: Thank you for updating this and choosing to support both at first. #backwards-compatible


##### Output

Expand Down
14 changes: 10 additions & 4 deletions internal/pkg/platform/localserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ type LocalServer struct {
token string
localHostedContext LocalHostedContext
cliConfig hooks.SDKCLIConfig
appPath string
Connection WebSocketConnection
delegateCmd hooks.ShellCommand // track running delegated process
delegateCmdMutex sync.Mutex // protect concurrent access
Expand Down Expand Up @@ -279,11 +280,16 @@ func (r *LocalServer) stopDelegateProcess(ctx context.Context) {
// connection for running app locally to script hook start
func (r *LocalServer) StartDelegate(ctx context.Context) error {
// Set up hook execution options
env := map[string]string{
"SLACK_CLI_XAPP": r.token,
"SLACK_CLI_XOXB": r.localHostedContext.BotAccessToken,
}
if r.appPath != "" {
env["SLACK_APP_PATH"] = r.appPath
env["SLACK_CLI_CUSTOM_FILE_PATH"] = r.appPath
}
var sdkManagedConnectionStartHookOpts = hooks.HookExecOpts{
Env: map[string]string{
"SLACK_CLI_XAPP": r.token,
"SLACK_CLI_XOXB": r.localHostedContext.BotAccessToken,
},
Env: env,
Exec: hooks.ShellExec{},
Hook: r.clients.SDKConfig.Hooks.Start,
}
Expand Down
30 changes: 16 additions & 14 deletions internal/pkg/platform/localserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,14 @@ func Test_LocalServer_Start(t *testing.T) {
TeamID: "justiceleague",
}
server := LocalServer{
clients,
"ABC123",
localContext,
clients.SDKConfig,
conn,
nil,
sync.Mutex{},
clients: clients,
token: "ABC123",
localHostedContext: localContext,
cliConfig: clients.SDKConfig,
appPath: "",
Connection: conn,
delegateCmd: nil,
delegateCmdMutex: sync.Mutex{},
}
if tc.fakeDialer != nil {
orig := *WebsocketDialerDial
Expand Down Expand Up @@ -349,13 +350,14 @@ func Test_LocalServer_Listen(t *testing.T) {
TeamID: "justiceleague",
}
server := LocalServer{
clients,
"ABC123",
localContext,
clients.SDKConfig,
conn,
nil,
sync.Mutex{},
clients: clients,
token: "ABC123",
localHostedContext: localContext,
cliConfig: clients.SDKConfig,
appPath: "",
Connection: conn,
delegateCmd: nil,
delegateCmdMutex: sync.Mutex{},
}
tc.Test(t, ctx, clientsMock, &server, conn)
})
Expand Down
2 changes: 2 additions & 0 deletions internal/pkg/platform/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type RunArgs struct {
Activity bool
ActivityLevel string
App types.App
AppPath string
Auth types.SlackAuth
Cleanup bool
ShowTriggers bool
Expand Down Expand Up @@ -126,6 +127,7 @@ func Run(ctx context.Context, clients *shared.ClientFactory, runArgs RunArgs) (t
token: localInstallResult.APIAccessTokens.AppLevel,
localHostedContext: localHostedContext,
cliConfig: cliConfig,
appPath: runArgs.AppPath,
Connection: nil,
}

Expand Down
Loading