Skip to content

Commit 5556e32

Browse files
committed
Explain complex Go test cases
Signed-off-by: Kaylee Lubick <klubick@nvidia.com>
1 parent 0071732 commit 5556e32

21 files changed

Lines changed: 78 additions & 0 deletions

‎scripts/inference-dispatcher/dispatcher_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ func configuredTestClient(t *testing.T, args []string, baseURL string, stderr *b
5151
return cfg, client
5252
}
5353

54+
// The inventory puts an embedding model first and chat models out of order.
55+
// Default selection must find a generation-capable model rather than use list order.
5456
func TestOmittedModel_SelectsAvailableGenerationModel(t *testing.T) {
5557
var receivedModel string
5658
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -145,6 +147,8 @@ func TestLMStudio_FallsBackToOpenAIInventory(t *testing.T) {
145147
assert.Contains(t, stdout.String(), "live-model", "selected model missing from output")
146148
}
147149

150+
// llama.cpp exposes OpenAI inventory and chat routes rather than Ollama's native
151+
// API. Auto-selection and dispatch must agree on that protocol.
148152
func TestLlamaCPP_UsesOpenAIInventoryAndChat(t *testing.T) {
149153
type observedRequest struct {
150154
method string

‎services/nvpair-cluster-manager/admission_migration_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ func TestLegacyPairingInfo_MapsToFirstAdmission(t *testing.T) {
3333
require.Error(t, err, "v2 pairing info without admission epoch was accepted")
3434
}
3535

36+
// Old persisted pins lack admission metadata. Restart must bind them to the
37+
// current cluster so existing members remain trusted and removable while offline.
3638
func TestRestart_MigratesLegacyPinnedMemberAdmission(t *testing.T) {
3739
dir := t.TempDir()
3840
m := testManagerAt(t, dir, 15124)

‎services/nvpair-engine-manager/executor_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ func TestEngineHTTPClients_BoundResponseHeaderWait(t *testing.T) {
7676
require.Equal(t, ollamaLoadResponseHeaderTimeout, responseHeaderTimeout(t, ex.ollamaLoadClient), "Ollama load response-header timeout")
7777
}
7878

79+
// A cold Ollama model load can delay response headers. Only that action gets the
80+
// longer budget, so unrelated operations still detect an unresponsive engine quickly.
7981
func TestOnlyOllamaRunModel_UsesSlowResponseHeaderBudget(t *testing.T) {
8082
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
8183
time.Sleep(100 * time.Millisecond)
@@ -229,6 +231,8 @@ func TestEngineLifecycle_DetectsStartsReportsHealthAndStops(t *testing.T) {
229231
require.False(t, st.Running, "expected stopped (%v)", st)
230232
}
231233

234+
// Another application can claim the port after a managed process crashes.
235+
// Restart must recognize that owner instead of spawning a competing process.
232236
func TestCrashThenExternalService_IsAdoptedWithoutRespawn(t *testing.T) {
233237
port, err := freePort()
234238
require.NoError(t, err)
@@ -595,6 +599,8 @@ func TestAdoptedEngine_ReportsObservedLiveness(t *testing.T) {
595599
require.False(t, st.Healthy, "closed external listener remained present (%v)", st)
596600
}
597601

602+
// The proxy also answers health requests on loopback. The identity probe must
603+
// distinguish it from Ollama, or forwarding can route requests back into the proxy.
598604
func TestProxyFacade_CannotIdentifyAsOllama(t *testing.T) {
599605
var probes atomic.Int32
600606
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -636,6 +642,8 @@ func TestProxyFacade_CannotIdentifyAsOllama(t *testing.T) {
636642
require.NotEqual(t, int32(0), probes.Load(), "engine identity probe did not carry the proxy sentinel")
637643
}
638644

645+
// A successful earlier identity check does not identify a new listener on the
646+
// same port. Losing that identity must block adoption and installation.
639647
func TestPreviouslyAdoptedForeignReplacement_FailsClosed(t *testing.T) {
640648
var isOllama atomic.Bool
641649
isOllama.Store(true)

‎services/nvpair-engine-manager/models_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616
"github.com/stretchr/testify/require"
1717
)
1818

19+
// Engines use different inventory shapes and loaded-model markers. Only rows
20+
// matching the configured path and filter may become model identifiers.
1921
func TestExtractStrings_ReadsConfiguredResultPath(t *testing.T) {
2022
tests := []struct {
2123
name string

‎services/nvpair-engine-manager/pull_llamacpp_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ func TestLlamaCPPPull_PreservesOtherDownloadWhenStartIsRejected(t *testing.T) {
286286
test("negative acknowledgement", http.StatusOK, `{"success":false}`)
287287
}
288288

289+
// Cancellation can race an unreadable start acknowledgement. Without proof that
290+
// this request started the download, cleanup could unload another caller's model.
289291
func TestLlamaCPPPullUnconfirmedStart_DoesNotUnload(t *testing.T) {
290292
test := func(name string, respond func(*testing.T, http.ResponseWriter, *http.Request)) {
291293
t.Run(name, func(t *testing.T) {

‎services/nvpair-engine-manager/pull_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ func newHTTPPullTestExecutor(t *testing.T, server *httptest.Server, action Actio
8888
return executor
8989
}
9090

91+
// A fast download can finish as soon as the start request arrives. Subscribe
92+
// first so its terminal event cannot be missed and leave the caller waiting.
9193
func TestPullModelLlamaCPP_SubscribesToProgressBeforeStartingDownload(t *testing.T) {
9294
const model = "owner/repo:Q4_K_M"
9395
subscribed := make(chan struct{})
@@ -169,6 +171,8 @@ func TestPullModelLlamaCPP_SubscribesToProgressBeforeStartingDownload(t *testing
169171
require.Equal(t, 100, events[1].Percent)
170172
}
171173

174+
// The complete download deliberately exceeds one idle interval. Advancing bytes
175+
// must extend the deadline so slow progress is not mistaken for a stalled download.
172176
func TestPullModelOllamaAdvancingProgress_RefreshesTimeout(t *testing.T) {
173177
const (
174178
idleTimeout = 400 * time.Millisecond
@@ -216,6 +220,8 @@ func TestPullModelOllamaAdvancingProgress_RefreshesTimeout(t *testing.T) {
216220
require.Contains(t, string(result), `"status":"success"`, "pull must report terminal success")
217221
}
218222

223+
// The SSE stream keeps advancing beyond the initial idle deadline. Each progress
224+
// update must renew that budget instead of imposing a total download time limit.
219225
func TestPullModelLlamaCPPAdvancingProgress_RefreshesTimeout(t *testing.T) {
220226
const (
221227
model = "owner/repo:Q4_K_M"

‎services/nvpair-engine-manager/settings_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,8 @@ const (
275275
restartsProcess
276276
)
277277

278+
// Changing only the proxy port must preserve a running engine, while engine
279+
// launch changes require a restart. Editing stopped engines must not start them.
278280
func TestConfigureLaunch_AppliesSettingsForEachEngineLifecycleState(t *testing.T) {
279281
test := func(name string, want lifecycle, edit func(*testing.T, *settings.Request)) {
280282
t.Run(name, func(t *testing.T) {
@@ -443,6 +445,8 @@ func TestSettings_ValidatesAndNormalizesCORSOrigins(t *testing.T) {
443445
reject("wildcard in origin list", "http://localhost,*", "every origin")
444446
}
445447

448+
// Only controls declared by the manifest carry CORS meaning. Treating arbitrary
449+
// engine flags as CORS settings could reject valid options or bypass declared checks.
446450
func TestSettingsCORS_IsOptionalAndIndependentOfEngine(t *testing.T) {
447451
setup := func(t *testing.T, corsEnv, corsFlag, prefix, suffix string) (*Executor, settings.Request, settings.Preview) {
448452
t.Helper()

‎services/nvpair-engine-manager/settingsremote_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ func TestNotifyPeerSettings_ForwardsEverySupportedEngine(t *testing.T) {
8787
}
8888
}
8989

90+
// Two pinned peers observe the same mutation, then one loses its pin. Revocation
91+
// must close its existing stream as well as reject new unauthenticated requests.
9092
func TestPairedSettings_PushesChangesAndRevokesObservers(t *testing.T) {
9193
a, certA := settingsMesh(t, "a")
9294
b, certB := settingsMesh(t, "b")
@@ -243,6 +245,8 @@ func TestSettingsRemoteOnlyCORS_IsLocalOnly(t *testing.T) {
243245
}
244246
}
245247

248+
// Replies arrive in reverse request order to expose accidental FIFO matching.
249+
// Cancellation must also remove its waiter so a later reply cannot retain it.
246250
func TestSettingsRelay_CorrelatesResponsesAndCleansUpCancelledRequests(t *testing.T) {
247251
sent := make(chan settings.Relay, 2)
248252
canceled := make(chan string, 1)

‎services/nvpair-job-scheduler/telemetry_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
"nvpair-shared/schedulerwire"
1616
)
1717

18+
// Dropping to a lower pressure band requires more headroom than entering it.
19+
// This prevents small utilization fluctuations from repeatedly changing node order.
1820
func TestTelemetryPressure_UsesBandsAndDownwardHysteresis(t *testing.T) {
1921
bands := []struct {
2022
utilization float64

‎services/nvpair-node-scanner/node_info_outage_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ func TestNodeInfoTransport_BoundsConnectionsPerHost(t *testing.T) {
9494
require.Equal(t, 1, transport.MaxIdleConnsPerHost)
9595
}
9696

97+
// Two requests share one connection and each fits its own timeout. Time spent
98+
// waiting for that connection must not consume the second request's response budget.
9799
func TestNodeInfoFetches_UseIndependentRequestBudgets(t *testing.T) {
98100
const responseDelay = 200 * time.Millisecond
99101
var requests atomic.Int32

0 commit comments

Comments
 (0)