Skip to content

Commit 26d315c

Browse files
Use idiomatic httptest handler failures in labs installer tests (#5623)
Follow-up to a review comment on #5559. `t.FailNow()` is documented as only legal on the goroutine running the test. The labs installer tests call it from inside `httptest` handler goroutines as the "unexpected request" fallback, where it only runs `runtime.Goexit` on the handler goroutine: the test is not actually failed and the client sees a dropped connection, which surfaces as a confusing error rather than a clear message. This replaces the four handler-goroutine occurrences with the idiomatic shape — `t.Errorf(...)` to record the failure on the test, plus `http.Error(w, "unexpected request", http.StatusInternalServerError)` to return a clear response. The one remaining `t.FailNow()` is on the test goroutine (a `t.Logf` / `t.FailNow` pair after the runner returns) and is collapsed to `t.Fatal`. Test-only change; no user-facing behavior changes. This pull request and its description were written by Isaac, an AI coding agent.
1 parent fefe317 commit 26d315c

1 file changed

Lines changed: 9 additions & 10 deletions

File tree

cmd/labs/project/installer_test.go

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,8 @@ func TestInstallerWorksForReleases(t *testing.T) {
193193
})
194194
return
195195
}
196-
t.Logf("Requested: %s", r.URL.Path)
197-
t.FailNow()
196+
t.Errorf("unexpected request: %s", r.URL.Path)
197+
http.Error(w, "unexpected request", http.StatusInternalServerError)
198198
}))
199199
defer server.Close()
200200

@@ -262,8 +262,8 @@ func TestOfflineInstallerWorksForReleases(t *testing.T) {
262262
})
263263
return
264264
}
265-
t.Logf("Requested: %s", r.URL.Path)
266-
t.FailNow()
265+
t.Errorf("unexpected request: %s", r.URL.Path)
266+
http.Error(w, "unexpected request", http.StatusInternalServerError)
267267
}))
268268
defer server.Close()
269269

@@ -362,8 +362,8 @@ func TestInstallerWorksForDevelopment(t *testing.T) {
362362
})
363363
return
364364
}
365-
t.Logf("Requested: %s", r.URL.Path)
366-
t.FailNow()
365+
t.Errorf("unexpected request: %s", r.URL.Path)
366+
http.Error(w, "unexpected request", http.StatusInternalServerError)
367367
}))
368368
defer server.Close()
369369

@@ -453,8 +453,8 @@ func TestUpgraderWorksForReleases(t *testing.T) {
453453
})
454454
return
455455
}
456-
t.Logf("Requested: %s", r.URL.Path)
457-
t.FailNow()
456+
t.Errorf("unexpected request: %s", r.URL.Path)
457+
http.Error(w, "unexpected request", http.StatusInternalServerError)
458458
}))
459459
defer server.Close()
460460

@@ -496,7 +496,6 @@ func TestUpgraderWorksForReleases(t *testing.T) {
496496
}
497497
}
498498
if !pi {
499-
t.Logf(`Expected stub command 'python[\S]+ -m pip install --upgrade --upgrade-strategy eager .' not found`)
500-
t.FailNow()
499+
t.Fatal(`Expected stub command 'python[\S]+ -m pip install --upgrade --upgrade-strategy eager .' not found`)
501500
}
502501
}

0 commit comments

Comments
 (0)