Skip to content

Commit 937cf6c

Browse files
authored
test(services): isolate test app-data state (#95)
## Description Fixes #48. Some Go tests inherited the developer's PAIR app-data directory. A saved LM Studio port file could affect a later run, and a test could overwrite the developer's saved port. `develop` replaced `services/lmstudio-proxy` and `services/ollama-proxy` with `services/nvpair-proxy`, so this branch now merges `develop` and ports the change to the unified proxy: - `services/tests` sets a private configuration root for every child binary it starts. A new test starts a real `nvpair-proxy`, enables the LM Studio facade, sends `lmstudio:set-port`, and checks that the port file is written below that root. - The `nvpair-proxy` end-to-end failover test redirects the configuration root before it starts its child process. Both set the variables after `go build`. On Linux and macOS, Go derives its module and build caches from `HOME`, so redirecting it earlier would build into the test directory. ## Release intent <!-- pair-release-intent:v1 --> ### Changelog title n/a ### Changelog body n/a ### Bumps - services: none - nvpair-cluster-manager: none - nvpair-engine-manager: none - nvpair-errors: none - nvpair-job-scheduler: none - nvpair-manual-nodes: none - nvpair-node-info: none - nvpair-node-scanner: none - nvpair-node-settings: none - nvpair-proxy: none - nvpair-tui: none - nvpair-ui-broker: none - nvpair-workload-manager: none <!-- /pair-release-intent:v1 --> ## Scope Included: - Test setup in `services/tests/main_test.go` and `services/nvpair-proxy/e2e_test.go`. - One cross-process regression test for LM Studio port persistence. Excluded: - Production app-directory behavior, flags, dependencies, APIs, documentation, and version files. - The earlier `nvpair-ui-broker` test file. `services/nvpair-ui-broker/testmain_test.go` on `develop` already isolates that package, and a second `TestMain` would not compile. The merge removes it. ## Validation - `go test . -run TestE2EFailoverOverRealBinary -count=2` in `services/nvpair-proxy` passed on Windows 11 with Go 1.26.5. - `go test . -run TestLMStudioFacadeChildPersistsUnderPrivateBase -count=2` in `services/tests` passed on the same machine. - `go test . -count=1` passed in `services/nvpair-proxy` and in `services/tests`. The only skips were the opt-in CORS tests and `TestBrokerShutsDownOnSignal`. - The real `%LOCALAPPDATA%\Nvidia Corporation\Personal AI Router` directory did not exist before these runs and did not exist after them. - On Linux (golang:1.26 container, non-root user), both focused tests passed twice and the full `nvpair-proxy` package passed. With the variable loop removed, the new test fails with "outside test config base". - `node scripts/spdx-headers.mjs`, `git diff --check`, and `scripts/release-intent/validate_pr.py` on this description passed. ## Risk This change affects test setup only. Compiled binaries and normal PAIR configuration resolution are unchanged. ## Checklist - [x] I have read the [Contributing Guidelines](https://github.com/NVIDIA/Personal-AI-Router/blob/main/CONTRIBUTING.md). - [x] Every commit is signed off (`git commit -s`), certifying the [Developer Certificate of Origin](https://developercertificate.org/). - [x] New or existing tests cover the change. - [x] Relevant documentation is updated. No documentation change is needed because users see no behavior change. - [x] I checked the diff, changed filenames, and commit messages for credentials, private data, internal URLs, internal issue identifiers, and generated artifacts. - [x] I recorded the validation commands and results above. - [x] I declared version bumps in the release-intent block above. `services/versions.json` is written by automation — do not edit it by hand. --------- Signed-off-by: Som Tripathi <somtri@iastate.edu>
1 parent be5e4fb commit 937cf6c

2 files changed

Lines changed: 123 additions & 0 deletions

File tree

‎services/nvpair-proxy/e2e_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,11 @@ func e2eSplitHostPort(t *testing.T, serverURL string) (string, int) {
279279
func TestE2EFailoverOverRealBinary(t *testing.T) {
280280
forEachEngine(t, func(t *testing.T, tc engineCase) {
281281
bin := proxyBinary(t)
282+
// The child inherits this environment, so a persisted port it loads on
283+
// facade/enable comes from a temp dir, never the developer's config.
284+
// It runs after the build, because go derives its module and build
285+
// caches from these variables.
286+
redirectConfigDir(t)
282287

283288
var gotBody string
284289
busy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

‎services/tests/main_test.go‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@ import (
1212
"os/exec"
1313
"path/filepath"
1414
"runtime"
15+
"strings"
1516
"testing"
1617
"time"
1718

19+
"nvpair-shared/appdir"
20+
"nvpair-shared/engines"
1821
"nvpair-shared/jsonrpc"
1922
)
2023

@@ -30,6 +33,7 @@ var (
3033
manualNodesBin string
3134
clusterMgrBin string
3235
schedulerBin string
36+
testsConfigBase string
3337
)
3438

3539
func TestMain(m *testing.M) {
@@ -135,6 +139,19 @@ func TestMain(m *testing.M) {
135139
log.Fatalf("build nvpair-job-scheduler: %v", err)
136140
}
137141

142+
// Every child binary inherits this environment, so its persisted state
143+
// (ports, workloads, settings) lands under tmpDir instead of the
144+
// developer's real config. These are the variables appdir resolves
145+
// through on each platform. They are set after the builds, because go
146+
// derives its module and build caches from these variables.
147+
testsConfigBase = filepath.Join(tmpDir, "config")
148+
for _, key := range []string{"XDG_CONFIG_HOME", "HOME", "APPDATA", "LOCALAPPDATA"} {
149+
if err := os.Setenv(key, testsConfigBase); err != nil {
150+
os.RemoveAll(tmpDir)
151+
log.Fatalf("set %s: %v", key, err)
152+
}
153+
}
154+
138155
code := m.Run()
139156
os.RemoveAll(tmpDir)
140157
os.Exit(code)
@@ -209,3 +226,104 @@ func waitForResponse(t *testing.T, ch <-chan jsonrpc.Message, timeout time.Durat
209226
}
210227
return jsonrpc.Message{}
211228
}
229+
230+
// codeFacadeBindFailed mirrors the constant of the same name in nvpair-proxy:
231+
// facade/enable answers with it when the port was taken before it could bind.
232+
const codeFacadeBindFailed = -32010
233+
234+
// requestOnFreePort sends the request that build makes for a free port and
235+
// returns the port the proxy accepted. freePort closes its probe before the
236+
// proxy binds, so another process can take the port first. A request refused
237+
// for that reason is retried on a new port; any other error fails the test.
238+
func requestOnFreePort(t *testing.T, w io.Writer, msgs <-chan jsonrpc.Message, timeout time.Duration, build func(port int) map[string]any) int {
239+
t.Helper()
240+
for attempt := 0; attempt < 8; attempt++ {
241+
port := freePort(t)
242+
req := build(port)
243+
sendLine(t, w, req)
244+
resp := waitForResponse(t, msgs, timeout)
245+
if resp.Error == nil {
246+
return port
247+
}
248+
if !isBindRace(resp.Error) {
249+
t.Fatalf("%s failed: %v", req["method"], resp.Error)
250+
}
251+
}
252+
t.Fatal("every probed port was taken before the proxy could bind it")
253+
return 0
254+
}
255+
256+
// isBindRace reports a request the proxy refused only because the port was
257+
// taken. facade/enable says so with codeFacadeBindFailed; set-port has no
258+
// dedicated code, so its bind error is matched by message.
259+
func isBindRace(e *jsonrpc.RPCError) bool {
260+
return e.Code == codeFacadeBindFailed || strings.HasPrefix(e.Message, "failed to bind port")
261+
}
262+
263+
// TestLMStudioFacadeChildPersistsUnderPrivateBase proves a real proxy child
264+
// writes its persisted LM Studio port under the TestMain config base, so no
265+
// cross-process test can clobber the developer's saved port.
266+
func TestLMStudioFacadeChildPersistsUnderPrivateBase(t *testing.T) {
267+
cmd := exec.Command(proxyBin)
268+
stdin, err := cmd.StdinPipe()
269+
if err != nil {
270+
t.Fatal(err)
271+
}
272+
stdout, err := cmd.StdoutPipe()
273+
if err != nil {
274+
t.Fatal(err)
275+
}
276+
if err := cmd.Start(); err != nil {
277+
t.Fatal(err)
278+
}
279+
defer func() {
280+
_ = stdin.Close()
281+
_ = cmd.Process.Kill()
282+
_ = cmd.Wait()
283+
}()
284+
msgs := startMsgReader(stdout)
285+
286+
requestOnFreePort(t, stdin, msgs, 10*time.Second, func(port int) map[string]any {
287+
return map[string]any{
288+
"jsonrpc": "2.0",
289+
"id": 1,
290+
"method": "facade/enable",
291+
"params": map[string]any{
292+
"engine": "lmstudio",
293+
"port": port,
294+
"ignorePersistedPort": true,
295+
},
296+
}
297+
})
298+
299+
persistedPort := requestOnFreePort(t, stdin, msgs, 5*time.Second, func(port int) map[string]any {
300+
return map[string]any{
301+
"jsonrpc": "2.0",
302+
"id": 2,
303+
"method": engines.AddressMethod("lmstudio", "set-port"),
304+
"params": map[string]any{"port": port},
305+
}
306+
})
307+
308+
path, err := appdir.Path("lmstudio-proxy-port.json")
309+
if err != nil {
310+
t.Fatal(err)
311+
}
312+
rel, err := filepath.Rel(testsConfigBase, path)
313+
if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
314+
t.Fatalf("lmstudio port path %q is outside test config base %q", path, testsConfigBase)
315+
}
316+
data, err := os.ReadFile(path)
317+
if err != nil {
318+
t.Fatalf("read persisted lmstudio port: %v", err)
319+
}
320+
var saved struct {
321+
Port int `json:"port"`
322+
}
323+
if err := json.Unmarshal(data, &saved); err != nil {
324+
t.Fatalf("parse persisted lmstudio port: %v", err)
325+
}
326+
if saved.Port != persistedPort {
327+
t.Fatalf("persisted lmstudio port = %d, want %d", saved.Port, persistedPort)
328+
}
329+
}

0 commit comments

Comments
 (0)