Repository navigation
test(harness): add e2e tests for thinking, tool calls, web search, and model switching - #183
FredLiu876 wants to merge 2 commits into
Conversation
…d model switching
|
|
There was a problem hiding this comment.
🟡 Changes recommended
Assertion gaps and setup-error handling can hide regressions, while Codex web-search detection needs correction.
6 open issues
What changed in this PR
Adds end-to-end coverage for real harness binaries using configurations generated by grid setup, including regression coverage for Pi’s adaptive-thinking integration.
Changes:
- Adds thinking, file-tool, web-search, and model-switching scenarios for Claude Code, Codex, and Pi.
- Adds shared helpers for configuration discovery, isolated harness state, bounded subprocess execution, and output parsing.
- Gates execution on credentials, installed binaries, and available route capabilities without changing product code.
| File | Description |
|---|---|
internal/e2e-tests/harness_test.go |
Adds harness behavior tests and session-continuity checks. |
internal/e2e-tests/harness_helpers_test.go |
Adds setup, configuration parsing, process management, and assertion helpers. |
Review effort: Balanced
| t.Skipf("the grid routes API (/v1/grid/routes) is not deployed on this remote (%s); harness e2e needs a remote with the grid routes feature enabled", remoteURL) | ||
| case strings.Contains(msg, "no routes"), | ||
| strings.Contains(msg, "no accessible routes"), | ||
| strings.Contains(msg, "no usable model metadata"), |
| } | ||
| lastErr = err | ||
| } | ||
| t.Skipf("none of the org's %d teams have routes %s can call; last probe error: %v", len(listed.Teams), name, lastErr) |
| if tools, ok := data["tools"].(map[string]any); ok { | ||
| switch search := tools["web_search"].(type) { | ||
| case bool: | ||
| h.codexWebSearch = search | ||
| case string: | ||
| h.codexWebSearch = search != "" && search != "disabled" && search != "false" | ||
| } | ||
| } |
| t.Helper() | ||
| data, err := os.ReadFile(filepath.Join(dir, name)) | ||
| require.NoError(t, err, "reading %s, which the harness was asked to produce", name) | ||
| return strings.TrimRight(string(data), "\n") |
| require.Zero(t, piErrorTurns(objects), "pi ended a turn with an API error; stdout:\n%s\nstderr:\n%s", stdout, stderr) | ||
| reply := piAssistantText(objects) | ||
| require.Contains(t, reply, harnessThinkingMarker, "pi's final reply lacks the marker; stdout:\n%s\nstderr:\n%s", stdout, stderr) | ||
| step(t, "pi thinking seen: %v", piThinkingSeen(objects)) |
| kind, _ := object["type"].(string) | ||
| if strings.Contains(kind, "web_search") { |
There was a problem hiding this comment.
Have not reviewed in full yet (will wait until copilot stuff looked at if needed), but can you confirm that this communicates with everything via the CLI directly the same way a user can? Also, is this a superset of what is in the existing e2e-ish harness/grid tests in the internal/cmd area? And if so, can those be removed and can the harness install step of CI be moved to as part of this e2e test run and not general purpose? One of my concerns of doing that however is that we may want to run across OS's which this test does not do, so we should discuss how best to do that.
Adds e2e tests that run the real claude-code, codex, and pi harness binaries against real Baseten routes through the harness configs
grid setupwrites.What's in the suite
Four behavior groups under
TestE2EHarness*(both new files are//go:build e2e):models.json, claude-code runs a route with an effort ceiling taken fromsettings.json. The suite asserts thinking actually engages (thinking blocks in the claude stream, no error turns in pi) and greps output forinvalid_request_errorand "not supported for this model", the exact failure signature of the original incident, so a contract break fails the test instead of degrading silently.task.txt, writeanswer.txtcontaining exactly its contents. Asserts tool calls fired and the file content is byte-exact.WebSearchand reply with a version number. The codex cell is a designed skip: harness setup writes noweb_searchsetting and codex's default web-search mode doesn't work against Baseten routes (the cell still runs if a config ever enables it).stage1.txtand remember a marker word under model A, then resume the session or thread under model B and write the recalled word tostage2.txt. Proves session continuity survives a mid-conversation model switch.Nothing is hardcoded. Models, routes, effort ceilings, endpoints, and web-search support are read back from the configs
grid setupactually wrote (settings.jsonfor claude-code,config.toml+model_catalog_jsonfor codex,models.jsonfor pi), so the suite runs in any org with harness-enabled routes. Below threshold it skips with the precise reason: fewer than two routes skips switching, no reasoning-capable Anthropic-format route skips pi thinking, no effort ceiling skips claude-code thinking.Safety and portability
BASETEN_CONFIG_DIR) and every scrap of harness state (HOME,CLAUDE_CONFIG_DIR,CODEX_HOME,PI_CODING_AGENT_DIR, the XDG dirs) lives in per-test temp roots, so tests can never touch a developer's or CI's real harness configs.org team listonly on failure, and a team-list error of any kind (e.g. a 404 on remotes without the app API) becomes a skip naming the cause rather than a CI failure.--use-session-credential, so the gate env key is used and nothing is minted.Validation
gofmt,go build ./...,go vet ./...,go vet -tags=e2e ./internal/e2e-tests/, and the unit suite (go test ./internal/...) all clean.go test -tags=e2e -count=1 -run TestE2EHarness): all green: 9 subtests with one designed skip (codex web search), ~100-105s per run, zero leftover temp roots.forceAdaptiveThinking, i.e. the test that would have caught the original incident.CI picks this up through the existing
BASETEN_E2E_TEST_API_KEY/BASETEN_E2E_TEST_REMOTE_URLsecrets (.github/workflows/ci.yml:69-70), e2e runs on ubuntu runners only, and tests skip when the key is absent.Out of scope