Skip to content

Commit ac708bb

Browse files
authored
Run client config CLI tests sequentially (#1092)
These are causing race conditions since they execute in parallel and need to set an environment variable in order to work. Another issue I found: Now that we enforce that the proxy port is not already used, tests would fail locally when the sample port of 8080 was already in used. Regarding the config env var race condition - In future, we may move the config to a database file, and we should address the lack of parameterization of the path at that point in time.
1 parent 633ea75 commit ac708bb

File tree

2 files changed

+14
-22
lines changed

2 files changed

+14
-22
lines changed

cmd/thv/app/client_test.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ func removeClientViaCLI(cmd *cobra.Command, client string) error {
2626
return cmd.Execute()
2727
}
2828

29-
func TestClientRegisterCmd(t *testing.T) {
30-
t.Parallel()
29+
func TestClientRegisterCmd(t *testing.T) { //nolint:paralleltest // Uses environment variables
3130
tempDir := t.TempDir()
3231
os.Setenv("XDG_CONFIG_HOME", tempDir)
3332

@@ -40,11 +39,7 @@ func TestClientRegisterCmd(t *testing.T) {
4039
assert.Contains(t, cfg.Clients.RegisteredClients, "vscode", "Client should be registered")
4140
}
4241

43-
// This test is failing due to a race condition.
44-
// We probably should refactor the code to accept a path instead of using the global config path.
45-
/*
46-
func TestClientRemoveCmd(t *testing.T) {
47-
t.Parallel()
42+
func TestClientRemoveCmd(t *testing.T) { //nolint:paralleltest // Uses environment variables
4843
tempDir := t.TempDir()
4944
os.Setenv("XDG_CONFIG_HOME", tempDir)
5045

@@ -62,10 +57,8 @@ func TestClientRemoveCmd(t *testing.T) {
6257
cfg := config.GetConfig()
6358
assert.NotContains(t, cfg.Clients.RegisteredClients, "vscode", "Client should be removed")
6459
}
65-
*/
6660

67-
func TestClientRegisterCmd_InvalidClient(t *testing.T) {
68-
t.Parallel()
61+
func TestClientRegisterCmd_InvalidClient(t *testing.T) { //nolint:paralleltest // Uses environment variables
6962
tempDir := t.TempDir()
7063
os.Setenv("XDG_CONFIG_HOME", tempDir)
7164

@@ -76,8 +69,7 @@ func TestClientRegisterCmd_InvalidClient(t *testing.T) {
7669
assert.True(t, strings.Contains(err.Error(), "invalid client type"))
7770
}
7871

79-
func TestClientRemoveCmd_InvalidClient(t *testing.T) {
80-
t.Parallel()
72+
func TestClientRemoveCmd_InvalidClient(t *testing.T) { //nolint:paralleltest // Uses environment variables
8173
tempDir := t.TempDir()
8274
os.Setenv("XDG_CONFIG_HOME", tempDir)
8375

pkg/runner/config_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -486,14 +486,14 @@ func TestRunConfig_WithStandardLabels(t *testing.T) {
486486
Name: "test-server",
487487
Image: "test-image",
488488
Transport: types.TransportTypeSSE,
489-
Port: 8080,
489+
Port: 60000,
490490
ContainerLabels: map[string]string{},
491491
},
492492
expected: map[string]string{
493493
"toolhive": "true",
494494
"toolhive-name": "test-server",
495495
"toolhive-transport": "sse",
496-
"toolhive-port": "8080",
496+
"toolhive-port": "60000",
497497
"toolhive-tool-type": "mcp",
498498
},
499499
},
@@ -577,7 +577,7 @@ func TestNewRunConfigFromFlags(t *testing.T) {
577577
permissionProfile := permissions.ProfileNone
578578
targetHost := "localhost"
579579
mcpTransport := "sse"
580-
port := 8080
580+
proxyPort := 60000
581581
targetPort := 9000
582582
envVars := []string{"TEST_ENV=test_value"}
583583
oidcIssuer := "https://issuer.example.com"
@@ -604,7 +604,7 @@ func TestNewRunConfigFromFlags(t *testing.T) {
604604
permissionProfile,
605605
targetHost,
606606
mcpTransport,
607-
port,
607+
proxyPort,
608608
targetPort,
609609
envVars,
610610
nil, // labels
@@ -653,7 +653,7 @@ func TestNewRunConfigFromFlags(t *testing.T) {
653653

654654
// Check transport settings
655655
assert.Equal(t, types.TransportTypeSSE, config.Transport, "Transport should be set to SSE")
656-
assert.Equal(t, port, config.Port, "ProxyPort should match")
656+
assert.Equal(t, proxyPort, config.Port, "ProxyPort should match")
657657
assert.Equal(t, targetPort, config.TargetPort, "TargetPort should match")
658658

659659
// Check OIDC config
@@ -677,7 +677,7 @@ func TestRunConfig_WriteJSON_ReadJSON(t *testing.T) {
677677
ContainerName: "test-container",
678678
BaseName: "test-base",
679679
Transport: types.TransportTypeSSE,
680-
Port: 8080,
680+
Port: 60000,
681681
TargetPort: 9000,
682682
Debug: true,
683683
ContainerLabels: map[string]string{
@@ -860,7 +860,7 @@ func TestNewRunConfigFromFlags_MetadataOverrides(t *testing.T) {
860860
permissions.ProfileNone,
861861
"localhost",
862862
tt.userTransport,
863-
8080, // port
863+
0, // port
864864
tt.userTargetPort,
865865
nil, // envVars
866866
nil, // labels
@@ -920,7 +920,7 @@ func TestNewRunConfigFromFlags_EnvironmentVariableTransportDependency(t *testing
920920
permissions.ProfileNone,
921921
"localhost",
922922
"sse", // This should result in MCP_TRANSPORT=sse in env vars
923-
8080,
923+
0,
924924
9000, // This should result in MCP_PORT=9000 in env vars
925925
[]string{"USER_VAR=value"},
926926
nil, // labels
@@ -973,7 +973,7 @@ func TestNewRunConfigFromFlags_CmdArgsMetadataPrepending(t *testing.T) {
973973
permissions.ProfileNone,
974974
"localhost",
975975
"",
976-
8080,
976+
0,
977977
0,
978978
nil,
979979
nil, // labels
@@ -1027,7 +1027,7 @@ func TestNewRunConfigFromFlags_VolumeProcessing(t *testing.T) {
10271027
permissions.ProfileNone, // Start with none profile
10281028
"localhost",
10291029
"",
1030-
8080,
1030+
0,
10311031
0,
10321032
nil,
10331033
nil, // labels

0 commit comments

Comments
 (0)