Skip to content

Commit 126416f

Browse files
mafredrimafredri
committed
better path handling
1 parent 64ee1da commit 126416f

File tree

2 files changed

+150
-27
lines changed

2 files changed

+150
-27
lines changed

‎cli/open.go

+22-17
Original file line numberDiff line numberDiff line change
@@ -103,28 +103,33 @@ func (r *RootCmd) openVSCode() *clibase.Cmd {
103103
}
104104
}
105105

106-
var directory string
107-
switch {
108-
case len(inv.Args) > 1:
109-
directory = inv.Args[1]
110-
111-
// We explicitly use `path.IsAbs` (vs `filepath`) because only
112-
// Windows absolute paths may start without "/".
113-
isUnixAndNotAbs := workspaceAgent.OperatingSystem != "windows" && !path.IsAbs(directory)
114-
115-
// Perhaps we could SSH in to expand the directory?
116-
if !insideThisWorkspace && (strings.HasPrefix(directory, "~") || isUnixAndNotAbs) {
117-
return xerrors.Errorf("directory path %q not supported, use an absolute path instead", directory)
118-
}
119-
if insideThisWorkspace {
106+
directory := workspaceAgent.ExpandedDirectory // Empty unless agent directory is set.
107+
if len(inv.Args) > 1 {
108+
d := inv.Args[1]
109+
110+
switch {
111+
case insideThisWorkspace:
120112
// TODO(mafredri): Return error if directory doesn't exist?
121-
directory, err = filepath.Abs(directory)
113+
directory, err = filepath.Abs(d)
122114
if err != nil {
123115
return xerrors.Errorf("expand directory: %w", err)
124116
}
117+
118+
case d == "~" || strings.HasPrefix(d, "~/"):
119+
return xerrors.Errorf("path %q requires expansion and is not supported, use an absolute path instead", d)
120+
121+
case workspaceAgent.OperatingSystem == "windows":
122+
// TODO(mafredri): For now we keep this simple instead of discerning out relative paths on Windows.
123+
directory = d
124+
125+
// Note that we use `path` instead of `filepath` since we want Unix behavior.
126+
case directory != "" && !path.IsAbs(d):
127+
directory = path.Join(directory, d)
128+
case path.IsAbs(d):
129+
directory = d
130+
default:
131+
return xerrors.Errorf("path %q not supported, use an absolute path instead", d)
125132
}
126-
case workspaceAgent.ExpandedDirectory != "":
127-
directory = workspaceAgent.ExpandedDirectory
128133
}
129134

130135
u, err := url.Parse("vscode://coder.coder-remote/open")

‎cli/open_test.go

+128-10
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import (
44
"net/url"
55
"os"
66
"path/filepath"
7-
"runtime"
8-
"strings"
97
"testing"
108

119
"github.com/stretchr/testify/assert"
@@ -20,11 +18,12 @@ import (
2018
"github.com/coder/coder/v2/testutil"
2119
)
2220

23-
func TestOpen(t *testing.T) {
21+
func TestOpenVSCode(t *testing.T) {
2422
t.Parallel()
2523

2624
agentName := "agent1"
27-
agentDir := "/tmp"
25+
agentDir, err := filepath.Abs(filepath.FromSlash("/tmp"))
26+
require.NoError(t, err)
2827
client, workspace, agentToken := setupWorkspaceForAgent(t, func(agents []*proto.Agent) []*proto.Agent {
2928
agents[0].Directory = agentDir
3029
agents[0].Name = agentName
@@ -66,12 +65,13 @@ func TestOpen(t *testing.T) {
6665
wantDir: agentDir,
6766
},
6867
{
69-
name: "relative path error",
68+
name: "ok relative path",
7069
args: []string{"--test.open-error", workspace.Name, "my/relative/path"},
71-
wantError: true,
70+
wantDir: filepath.Join(agentDir, filepath.FromSlash("my/relative/path")),
71+
wantError: false,
7272
},
7373
{
74-
name: "ok with abs path",
74+
name: "ok with absolute path",
7575
args: []string{"--test.open-error", workspace.Name, agentDir},
7676
wantDir: agentDir,
7777
},
@@ -140,9 +140,127 @@ func TestOpen(t *testing.T) {
140140
assert.Equal(t, workspace.Name, qp.Get("workspace"))
141141
assert.Equal(t, agentName, qp.Get("agent"))
142142
if tt.wantDir != "" {
143-
if runtime.GOOS == "windows" {
144-
tt.wantDir = strings.TrimPrefix(tt.wantDir, "/")
145-
}
143+
assert.Contains(t, qp.Get("folder"), tt.wantDir)
144+
} else {
145+
assert.Empty(t, qp.Get("folder"))
146+
}
147+
if tt.wantToken {
148+
assert.NotEmpty(t, qp.Get("token"))
149+
} else {
150+
assert.Empty(t, qp.Get("token"))
151+
}
152+
153+
w.RequireSuccess()
154+
})
155+
}
156+
}
157+
158+
func TestOpenVSCode_NoAgentDirectory(t *testing.T) {
159+
t.Parallel()
160+
161+
agentName := "agent1"
162+
client, workspace, agentToken := setupWorkspaceForAgent(t, func(agents []*proto.Agent) []*proto.Agent {
163+
agents[0].Name = agentName
164+
return agents
165+
})
166+
167+
_ = agenttest.New(t, client.URL, agentToken)
168+
_ = coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
169+
170+
insideWorkspaceEnv := map[string]string{
171+
"CODER": "true",
172+
"CODER_WORKSPACE_NAME": workspace.Name,
173+
"CODER_WORKSPACE_AGENT_NAME": agentName,
174+
}
175+
176+
wd, err := os.Getwd()
177+
require.NoError(t, err)
178+
179+
tests := []struct {
180+
name string
181+
args []string
182+
env map[string]string
183+
wantDir string
184+
wantToken bool
185+
wantError bool
186+
}{
187+
{
188+
name: "ok",
189+
args: []string{"--test.open-error", workspace.Name},
190+
},
191+
{
192+
name: "no agent dir error relative path",
193+
args: []string{"--test.open-error", workspace.Name, "my/relative/path"},
194+
wantDir: filepath.FromSlash("my/relative/path"),
195+
wantError: true,
196+
},
197+
{
198+
name: "ok with absolute path",
199+
args: []string{"--test.open-error", workspace.Name, "/home/coder"},
200+
wantDir: "/home/coder",
201+
},
202+
{
203+
name: "ok with token",
204+
args: []string{"--test.open-error", workspace.Name, "--generate-token"},
205+
wantToken: true,
206+
},
207+
// Inside workspace, does not require --test.open-error.
208+
{
209+
name: "ok inside workspace",
210+
env: insideWorkspaceEnv,
211+
args: []string{workspace.Name},
212+
},
213+
{
214+
name: "ok inside workspace relative path",
215+
env: insideWorkspaceEnv,
216+
args: []string{workspace.Name, "foo"},
217+
wantDir: filepath.Join(wd, "foo"),
218+
},
219+
{
220+
name: "ok inside workspace token",
221+
env: insideWorkspaceEnv,
222+
args: []string{workspace.Name, "--generate-token"},
223+
wantToken: true,
224+
},
225+
}
226+
227+
for _, tt := range tests {
228+
tt := tt
229+
t.Run(tt.name, func(t *testing.T) {
230+
t.Parallel()
231+
232+
inv, root := clitest.New(t, append([]string{"open", "vscode"}, tt.args...)...)
233+
clitest.SetupConfig(t, client, root)
234+
pty := ptytest.New(t)
235+
inv.Stdin = pty.Input()
236+
inv.Stdout = pty.Output()
237+
238+
ctx := testutil.Context(t, testutil.WaitLong)
239+
inv = inv.WithContext(ctx)
240+
for k, v := range tt.env {
241+
inv.Environ.Set(k, v)
242+
}
243+
244+
w := clitest.StartWithWaiter(t, inv)
245+
246+
if tt.wantError {
247+
w.RequireError()
248+
return
249+
}
250+
251+
me, err := client.User(ctx, codersdk.Me)
252+
require.NoError(t, err)
253+
254+
line := pty.ReadLine(ctx)
255+
u, err := url.ParseRequestURI(line)
256+
require.NoError(t, err, "line: %q", line)
257+
258+
qp := u.Query()
259+
assert.Equal(t, client.URL.String(), qp.Get("url"))
260+
assert.Equal(t, me.Username, qp.Get("owner"))
261+
assert.Equal(t, workspace.Name, qp.Get("workspace"))
262+
assert.Equal(t, agentName, qp.Get("agent"))
263+
if tt.wantDir != "" {
146264
assert.Contains(t, qp.Get("folder"), tt.wantDir)
147265
} else {
148266
assert.Empty(t, qp.Get("folder"))

0 commit comments

Comments
 (0)