Skip to content

Commit 05127e6

Browse files
ammarioammario
committed
Use tag system
1 parent cae381f commit 05127e6

File tree

5 files changed

+19
-65
lines changed

5 files changed

+19
-65
lines changed

‎Makefile

-20
Original file line numberDiff line numberDiff line change
@@ -639,26 +639,6 @@ test-postgres-docker:
639639
done
640640
.PHONY: test-postgres-docker
641641

642-
build/go_tests.json: $(GO_SRC_FILES)
643-
go test ./... -list="." -json > "$@"
644-
645-
test-timing: build/go_tests.json
646-
# The timing tests intentionally run without parallelism to avoid flakiness.
647-
function test_cmds {
648-
cat "$<" \
649-
| jq -r '
650-
select(.Action == "output" and (.Output | test("_Timing\n")))
651-
| "go test \(.Package) -run=^\(.Output)"
652-
' \
653-
| grep -v ^$$
654-
}
655-
while read cmd; do
656-
echo "running $$cmd"
657-
$$cmd
658-
done < <(test_cmds)
659-
660-
.PHONY: test-timing
661-
662642
test-clean:
663643
go clean -testcache
664644
.PHONY: test-clean

‎agent/agent.go

+7-41
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,9 @@ import (
1414
"net/http"
1515
"net/netip"
1616
"os"
17-
"os/exec"
1817
"os/user"
1918
"path/filepath"
2019
"reflect"
21-
"regexp"
2220
"sort"
2321
"strconv"
2422
"strings"
@@ -37,7 +35,6 @@ import (
3735

3836
"cdr.dev/slog"
3937
"github.com/coder/coder/agent/agentssh"
40-
"github.com/coder/coder/agent/usershell"
4138
"github.com/coder/coder/buildinfo"
4239
"github.com/coder/coder/coderd/database"
4340
"github.com/coder/coder/coderd/gitauth"
@@ -205,42 +202,7 @@ func (a *agent) runLoop(ctx context.Context) {
205202
}
206203
}
207204

208-
var (
209-
isCmdExeRe = regexp.MustCompile(`^(cmd|cmd\.exe)$`)
210-
)
211-
212-
func createMetadataCommand(ctx context.Context, script string) (*exec.Cmd, error) {
213-
// This is largely copied from agentssh, but for some reason the command
214-
// generated there always returns exit status 1 in Windows powershell.
215-
//
216-
// This function puts special PowerShell branching in place that fixes the issue,
217-
// but I'm hesitant on porting it to agentssh before understanding exactly what's
218-
// happening.
219-
currentUser, err := user.Current()
220-
if err != nil {
221-
return nil, xerrors.Errorf("get current user: %w", err)
222-
}
223-
username := currentUser.Username
224-
225-
shell, err := usershell.Get(username)
226-
if err != nil {
227-
return nil, xerrors.Errorf("get user shell: %w", err)
228-
}
229-
shellBase := filepath.Base(shell)
230-
231-
var args []string
232-
switch {
233-
case isCmdExeRe.MatchString(shellBase):
234-
args = append(args, "/c")
235-
default:
236-
// -c works for powershell and sh variants.
237-
args = append(args, "-c")
238-
}
239-
args = append(args, script)
240-
return exec.CommandContext(ctx, shell, args...), nil
241-
}
242-
243-
func (*agent) collectMetadata(ctx context.Context, md codersdk.WorkspaceAgentMetadataDescription) *codersdk.WorkspaceAgentMetadataResult {
205+
func (a *agent) collectMetadata(ctx context.Context, md codersdk.WorkspaceAgentMetadataDescription) *codersdk.WorkspaceAgentMetadataResult {
244206
var out bytes.Buffer
245207
result := &codersdk.WorkspaceAgentMetadataResult{
246208
// CollectedAt is set here for testing purposes and overrode by
@@ -250,7 +212,7 @@ func (*agent) collectMetadata(ctx context.Context, md codersdk.WorkspaceAgentMet
250212
// if it can guarantee the clocks are synchronized.
251213
CollectedAt: time.Now(),
252214
}
253-
cmd, err := createMetadataCommand(ctx, md.Script)
215+
cmd, err := a.sshServer.CreateCommand(ctx, md.Script, nil)
254216
if err != nil {
255217
result.Error = fmt.Sprintf("create cmd: %+v", err)
256218
return result
@@ -278,8 +240,12 @@ func (*agent) collectMetadata(ctx context.Context, md codersdk.WorkspaceAgentMet
278240
out.Truncate(bufLimit)
279241
}
280242

243+
// Important: if the command times out, we may see a misleading error like
244+
// "exit status 1", so it's important to include the context error.
245+
err = errors.Join(err, ctx.Err())
246+
281247
if err != nil {
282-
result.Error = fmt.Sprintf("run cmd (shell %v): %+v", cmd.Path, err)
248+
result.Error = fmt.Sprintf("run cmd: %+v", err)
283249
}
284250
result.Value = out.String()
285251
return result

‎agent/agent_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,7 @@ func TestAgent_Metadata(t *testing.T) {
10161016
{
10171017
Key: "greeting",
10181018
Interval: 1,
1019+
Timeout: 100,
10191020
Script: echoHello,
10201021
},
10211022
},
@@ -1085,7 +1086,7 @@ func TestAgentMetadata_Timing(t *testing.T) {
10851086
}
10861087

10871088
require.Equal(t, "hello\n", md["greeting"].Value)
1088-
require.Equal(t, "exit status 1", md["bad"].Error)
1089+
require.Equal(t, "run cmd: exit status 1", md["bad"].Error)
10891090

10901091
greetingByt, err := os.ReadFile(greetingPath)
10911092
require.NoError(t, err)

‎testutil/enable_timing.go

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//go:build timing
2+
3+
package testutil
4+
5+
var _ = func() any {
6+
timing = true
7+
return nil
8+
}()

‎testutil/timing.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package testutil
22

33
import (
4-
"flag"
54
"testing"
65
)
76

@@ -12,10 +11,10 @@ import (
1211
//
1312
// Eventually, we should run all timing tests in a self-hosted runner.
1413

15-
var timingFlag = flag.Bool("timing", false, "run timing-sensitive tests")
14+
var timing bool
1615

1716
func SkipIfNotTiming(t *testing.T) {
18-
if !*timingFlag {
17+
if !timing {
1918
t.Skip("skipping timing-sensitive test")
2019
}
2120
}

0 commit comments

Comments
 (0)