RomanNum3ral
/
gstack
mirror of https://github.com/garrytan/gstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

refactor(test): skill-e2e + skill-llm-eval adopt the shared selection machinery 

Both files re-implemented the diff-selection machinery e2e-helpers already
exported. The helper gained computeDiffSelection() (extracted, identical
behavior) and a trailing optional selection param on the *IfSelected helpers
(defaults preserve all 30+ existing importers). skill-e2e.test.ts drops ~120
duplicated lines; skill-llm-eval keeps its LLM_JUDGE_TOUCHFILES selection and
test.concurrent semantics via testConcurrentIfSelected.

Deliberate deltas, stated: skill-e2e.test.ts now honors the EVALS_TIER
intersection its local copy lacked (affects only direct bun test invocations
of that file — it matches no eval-script glob); its recordE2E gains the
helper's three diagnostic fields; skill-llm-eval sharded solo now runs
e2e-helpers' module-scope preflight it already ran in combined processes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-08-14 20:25:37 -07:00
parent 290c71046b
commit 593be14c49
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
3 changed files with 86 additions and 222 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
test/helpers/e2e-helpers.ts
View File

@ -32,26 +32,36 @@ export const evalsEnabled = !!process.env.EVALS;
// --- Diff-based test selection --- // --- Diff-based test selection ---
// When EVALS_ALL is not set, only run tests whose touchfiles were modified. // When EVALS_ALL is not set, only run tests whose touchfiles were modified.
// Set EVALS_ALL=1 to force all tests. Set EVALS_BASE to override base branch. // Set EVALS_ALL=1 to force all tests. Set EVALS_BASE to override base branch.
export let selectedTests: string[] | null = null; // null = run all
if (evalsEnabled && !process.env.EVALS_ALL) { /**
* Compute the diff-based selection for a touchfiles table. Returns null for
* "run all" (EVALS off, EVALS_ALL=1, or no diff vs the base branch — e.g. on
* main). Shared by this module (E2E_TOUCHFILES) and skill-llm-eval.test.ts
* (LLM_JUDGE_TOUCHFILES) so the selection logic exists exactly once.
*/
export function computeDiffSelection(
touchfiles: Record<string, string[]>,
label: string,
): string[] | null {
if (!evalsEnabled || process.env.EVALS_ALL) return null;
const baseBranch = process.env.EVALS_BASE const baseBranch = process.env.EVALS_BASE
|| detectBaseBranch(ROOT) || detectBaseBranch(ROOT)
|| 'main'; || 'main';
const changedFiles = getChangedFiles(baseBranch, ROOT); const changedFiles = getChangedFiles(baseBranch, ROOT);
// If changedFiles is empty (e.g., on main branch), run all
if (changedFiles.length === 0) return null;
if (changedFiles.length > 0) { const selection = selectTests(changedFiles, touchfiles, GLOBAL_TOUCHFILES);
const selection = selectTests(changedFiles, E2E_TOUCHFILES, GLOBAL_TOUCHFILES); process.stderr.write(`\n${label} selection (${selection.reason}): ${selection.selected.length}/${Object.keys(touchfiles).length} tests\n`);
selectedTests = selection.selected; if (selection.skipped.length > 0) {
process.stderr.write(`\nE2E selection (${selection.reason}): ${selection.selected.length}/${Object.keys(E2E_TOUCHFILES).length} tests\n`); process.stderr.write(` Skipped: ${selection.skipped.join(', ')}\n`);
if (selection.skipped.length > 0) {
process.stderr.write(` Skipped: ${selection.skipped.join(', ')}\n`);
}
process.stderr.write('\n');
} }
// If changedFiles is empty (e.g., on main branch), selectedTests stays null → run all process.stderr.write('\n');
return selection.selected;
} }
export let selectedTests: string[] | null = computeDiffSelection(E2E_TOUCHFILES, 'E2E'); // null = run all
// EVALS_TIER: filter tests by tier after diff-based selection. // EVALS_TIER: filter tests by tier after diff-based selection.
// 'gate' = gate tests only (CI default — blocks merge) // 'gate' = gate tests only (CI default — blocks merge)
// 'periodic' = periodic tests only (weekly cron / manual) // 'periodic' = periodic tests only (weekly cron / manual)
@ -72,9 +82,14 @@ if (evalsEnabled && process.env.EVALS_TIER) {
export const describeE2E = evalsEnabled ? describe : describe.skip; export const describeE2E = evalsEnabled ? describe : describe.skip;
/** Wrap a describe block to skip entirely if none of its tests are selected. */ /**
export function describeIfSelected(name: string, testNames: string[], fn: () => void) { * Wrap a describe block to skip entirely if none of its tests are selected.
const anySelected = selectedTests === null || testNames.some(t => selectedTests!.includes(t)); * `selected` defaults to this module's E2E selection (diff + EVALS_TIER);
* pass an explicit selection (e.g. computeDiffSelection over
* LLM_JUDGE_TOUCHFILES) to reuse the gating against a different table.
*/
export function describeIfSelected(name: string, testNames: string[], fn: () => void, selected: string[] | null = selectedTests) {
const anySelected = selected === null || testNames.some(t => selected.includes(t));
(anySelected ? describeE2E : describe.skip)(name, fn); (anySelected ? describeE2E : describe.skip)(name, fn);
} }
@ -272,14 +287,14 @@ if (evalsEnabled) {
} }
/** Skip an individual test if not selected (for multi-test describe blocks). */ /** Skip an individual test if not selected (for multi-test describe blocks). */
export function testIfSelected(testName: string, fn: () => Promise<void>, timeout: number) { export function testIfSelected(testName: string, fn: () => Promise<void>, timeout: number, selected: string[] | null = selectedTests) {
const shouldRun = selectedTests === null || selectedTests.includes(testName); const shouldRun = selected === null || selected.includes(testName);
(shouldRun ? test : test.skip)(testName, fn, timeout); (shouldRun ? test : test.skip)(testName, fn, timeout);
} }
/** Concurrent version — runs in parallel with other concurrent tests within the same describe block. */ /** Concurrent version — runs in parallel with other concurrent tests within the same describe block. */
export function testConcurrentIfSelected(testName: string, fn: () => Promise<void>, timeout: number) { export function testConcurrentIfSelected(testName: string, fn: () => Promise<void>, timeout: number, selected: string[] | null = selectedTests) {
const shouldRun = selectedTests === null || selectedTests.includes(testName); const shouldRun = selected === null || selected.includes(testName);
(shouldRun ? test.concurrent : test.skip)(testName, fn, timeout); (shouldRun ? test.concurrent : test.skip)(testName, fn, timeout);
} }

191
test/skill-e2e.test.ts
View File

@ -2,172 +2,50 @@ import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
import { runSkillTest } from './helpers/session-runner'; import { runSkillTest } from './helpers/session-runner';
import type { SkillTestResult } from './helpers/session-runner'; import type { SkillTestResult } from './helpers/session-runner';
import { outcomeJudge, callJudge } from './helpers/llm-judge'; import { outcomeJudge, callJudge } from './helpers/llm-judge';
import { EvalCollector, judgePassed } from './helpers/eval-store'; import { judgePassed } from './helpers/eval-store';
import type { EvalTestEntry } from './helpers/eval-store'; import type { EvalTestEntry } from './helpers/eval-store';
import { startTestServer } from '../browse/test/test-server'; import { startTestServer } from '../browse/test/test-server';
import { selectTests, detectBaseBranch, getChangedFiles, E2E_TOUCHFILES, GLOBAL_TOUCHFILES } from './helpers/touchfiles'; // Skip unless EVALS=1 (evalsEnabled/describeE2E). Diff-based selection,
import { spawnSync } from 'child_process'; // the EVALS_TIER filter, the API-reachability fail-fast ping, and the
import * as fs from 'fs'; // ~/.gstack pre-seed all run at e2e-helpers import time — see that module.
import * as path from 'path';
import * as os from 'os';
const ROOT = path.resolve(import.meta.dir, '..');
// Skip unless EVALS=1. Session runner strips CLAUDE* env vars to avoid nested session issues.
// //
// BLAME PROTOCOL: When an eval fails, do NOT claim "pre-existing" or "not related // BLAME PROTOCOL: When an eval fails, do NOT claim "pre-existing" or "not related
// to our changes" without proof. Run the same eval on main to verify. These tests // to our changes" without proof. Run the same eval on main to verify. These tests
// have invisible couplings — preamble text, SKILL.md content, and timing all affect // have invisible couplings — preamble text, SKILL.md content, and timing all affect
// agent behavior. See CLAUDE.md "E2E eval failure blame protocol" for details. // agent behavior. See CLAUDE.md "E2E eval failure blame protocol" for details.
const evalsEnabled = !!process.env.EVALS; import {
const describeE2E = evalsEnabled ? describe : describe.skip; ROOT,
evalsEnabled,
// --- Diff-based test selection --- describeE2E,
// When EVALS_ALL is not set, only run tests whose touchfiles were modified. selectedTests,
// Set EVALS_ALL=1 to force all tests. Set EVALS_BASE to override base branch. describeIfSelected,
let selectedTests: string[] | null = null; // null = run all testIfSelected,
createEvalCollector,
if (evalsEnabled && !process.env.EVALS_ALL) { recordE2E as recordE2EShared,
const baseBranch = process.env.EVALS_BASE finalizeEvalCollector,
|| detectBaseBranch(ROOT) runId,
|| 'main'; browseBin,
const changedFiles = getChangedFiles(baseBranch, ROOT); copyDirSync,
setupBrowseShims,
if (changedFiles.length > 0) { logCost,
const selection = selectTests(changedFiles, E2E_TOUCHFILES, GLOBAL_TOUCHFILES); dumpOutcomeDiagnostic,
selectedTests = selection.selected; hasApiKey,
process.stderr.write(`\nE2E selection (${selection.reason}): ${selection.selected.length}/${Object.keys(E2E_TOUCHFILES).length} tests\n`); } from './helpers/e2e-helpers';
if (selection.skipped.length > 0) { import { spawnSync } from 'child_process';
process.stderr.write(` Skipped: ${selection.skipped.join(', ')}\n`); import * as fs from 'fs';
} import * as path from 'path';
process.stderr.write('\n'); import * as os from 'os';
}
// If changedFiles is empty (e.g., on main branch), selectedTests stays null → run all
}
/** Wrap a describe block to skip entirely if none of its tests are selected. */
function describeIfSelected(name: string, testNames: string[], fn: () => void) {
const anySelected = selectedTests === null || testNames.some(t => selectedTests!.includes(t));
(anySelected ? describeE2E : describe.skip)(name, fn);
}
/** Skip an individual test if not selected (for multi-test describe blocks). */
function testIfSelected(testName: string, fn: () => Promise<void>, timeout: number) {
const shouldRun = selectedTests === null || selectedTests.includes(testName);
(shouldRun ? test : test.skip)(testName, fn, timeout);
}
// Eval result collector — accumulates test results, writes to ~/.gstack-dev/evals/ on finalize // Eval result collector — accumulates test results, writes to ~/.gstack-dev/evals/ on finalize
const evalCollector = evalsEnabled ? new EvalCollector('e2e') : null; const evalCollector = createEvalCollector('e2e');
// Unique run ID for this E2E session — used for heartbeat + per-run log directory /** Record a result into this file's collector (recording logic lives in e2e-helpers). */
const runId = new Date().toISOString().replace(/[:.]/g, '').replace('T', '-').slice(0, 15);
/** DRY helper to record an E2E test result into the eval collector. */
function recordE2E(name: string, suite: string, result: SkillTestResult, extra?: Partial<EvalTestEntry>) { function recordE2E(name: string, suite: string, result: SkillTestResult, extra?: Partial<EvalTestEntry>) {
// Derive last tool call from transcript for machine-readable diagnostics recordE2EShared(evalCollector, name, suite, result, extra);
const lastTool = result.toolCalls.length > 0
? `${result.toolCalls[result.toolCalls.length - 1].tool}(${JSON.stringify(result.toolCalls[result.toolCalls.length - 1].input).slice(0, 60)})`
: undefined;
evalCollector?.addTest({
name, suite, tier: 'e2e',
passed: result.exitReason === 'success' && result.browseErrors.length === 0,
duration_ms: result.duration,
cost_usd: result.costEstimate.estimatedCost,
transcript: result.transcript,
output: result.output?.slice(0, 2000),
turns_used: result.costEstimate.turnsUsed,
browse_errors: result.browseErrors,
exit_reason: result.exitReason,
timeout_at_turn: result.exitReason === 'timeout' ? result.costEstimate.turnsUsed : undefined,
last_tool_call: lastTool,
...extra,
});
} }
let testServer: ReturnType<typeof startTestServer>; let testServer: ReturnType<typeof startTestServer>;
let tmpDir: string; let tmpDir: string;
const browseBin = path.resolve(ROOT, 'browse', 'dist', 'browse');
/**
* Copy a directory tree recursively (files only, follows structure).
*/
function copyDirSync(src: string, dest: string) {
fs.mkdirSync(dest, { recursive: true });
for (const entry of fs.readdirSync(src, { withFileTypes: true })) {
const srcPath = path.join(src, entry.name);
const destPath = path.join(dest, entry.name);
if (entry.isDirectory()) {
copyDirSync(srcPath, destPath);
} else {
fs.copyFileSync(srcPath, destPath);
}
}
}
/**
* Set up browse shims (binary symlink, find-browse, remote-slug) in a tmpDir.
*/
function setupBrowseShims(dir: string) {
// Symlink browse binary
const binDir = path.join(dir, 'browse', 'dist');
fs.mkdirSync(binDir, { recursive: true });
if (fs.existsSync(browseBin)) {
fs.symlinkSync(browseBin, path.join(binDir, 'browse'));
}
// find-browse shim
const findBrowseDir = path.join(dir, 'browse', 'bin');
fs.mkdirSync(findBrowseDir, { recursive: true });
fs.writeFileSync(
path.join(findBrowseDir, 'find-browse'),
`#!/bin/bash\necho "${browseBin}"\n`,
{ mode: 0o755 },
);
// remote-slug shim (returns test-project)
fs.writeFileSync(
path.join(findBrowseDir, 'remote-slug'),
`#!/bin/bash\necho "test-project"\n`,
{ mode: 0o755 },
);
}
/**
* Print cost summary after an E2E test.
*/
function logCost(label: string, result: { costEstimate: { turnsUsed: number; estimatedTokens: number; estimatedCost: number }; duration: number }) {
const { turnsUsed, estimatedTokens, estimatedCost } = result.costEstimate;
const durationSec = Math.round(result.duration / 1000);
console.log(`${label}: $${estimatedCost.toFixed(2)} (${turnsUsed} turns, ${(estimatedTokens / 1000).toFixed(1)}k tokens, ${durationSec}s)`);
}
/**
* Dump diagnostic info on planted-bug outcome failure (decision 1C).
*/
function dumpOutcomeDiagnostic(dir: string, label: string, report: string, judgeResult: any) {
try {
const transcriptDir = path.join(dir, '.gstack', 'test-transcripts');
fs.mkdirSync(transcriptDir, { recursive: true });
const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
fs.writeFileSync(
path.join(transcriptDir, `${label}-outcome-${timestamp}.json`),
JSON.stringify({ label, report, judgeResult }, null, 2),
);
} catch { /* non-fatal */ }
}
// Fail fast if Anthropic API is unreachable — don't burn through 13 tests getting ConnectionRefused
if (evalsEnabled) {
const check = spawnSync('sh', ['-c', 'echo "ping" | claude -p --max-turns 1 --output-format stream-json --verbose --dangerously-skip-permissions'], {
stdio: 'pipe', timeout: 30_000,
});
const output = check.stdout?.toString() || '';
if (output.includes('ConnectionRefused') || output.includes('Unable to connect')) {
throw new Error('Anthropic API unreachable — aborting E2E suite. Fix connectivity and retry.');
}
}
describeIfSelected('Skill E2E tests', [ describeIfSelected('Skill E2E tests', [
'browse-basic', 'browse-snapshot', 'skillmd-setup-discovery', 'browse-basic', 'browse-snapshot', 'skillmd-setup-discovery',
@ -674,7 +552,6 @@ Important: The design checklist should catch issues like blacklisted fonts, smal
// --- B6/B7/B8: Planted-bug outcome evals --- // --- B6/B7/B8: Planted-bug outcome evals ---
// Outcome evals also need ANTHROPIC_API_KEY for the LLM judge // Outcome evals also need ANTHROPIC_API_KEY for the LLM judge
const hasApiKey = !!process.env.ANTHROPIC_API_KEY;
const describeOutcome = (evalsEnabled && hasApiKey) ? describe : describe.skip; const describeOutcome = (evalsEnabled && hasApiKey) ? describe : describe.skip;
// Wrap describeOutcome with selection — skip if no planted-bug tests are selected // Wrap describeOutcome with selection — skip if no planted-bug tests are selected
@ -3266,12 +3143,4 @@ Write your summary to ${benefitsDir}/benefits-summary.md`,
// Module-level afterAll — finalize eval collector after all tests complete // Module-level afterAll — finalize eval collector after all tests complete
afterAll(async () => { afterAll(() => finalizeEvalCollector(evalCollector));
if (evalCollector) {
try {
await evalCollector.finalize();
} catch (err) {
console.error('Failed to save eval results:', err);
}
}
});

66
test/skill-llm-eval.test.ts
View File

@ -10,53 +10,41 @@
* Cost: ~$0.05-0.15 per run (sonnet) * Cost: ~$0.05-0.15 per run (sonnet)
*/ */
import { describe, test, expect, afterAll } from 'bun:test'; import { afterAll, expect } from 'bun:test';
import Anthropic from '@anthropic-ai/sdk'; import Anthropic from '@anthropic-ai/sdk';
import * as fs from 'fs'; import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import { callJudge, judge } from './helpers/llm-judge'; import { callJudge, judge } from './helpers/llm-judge';
import type { JudgeScore } from './helpers/llm-judge'; import type { JudgeScore } from './helpers/llm-judge';
import { EvalCollector } from './helpers/eval-store'; import { LLM_JUDGE_TOUCHFILES } from './helpers/touchfiles';
import { selectTests, detectBaseBranch, getChangedFiles, LLM_JUDGE_TOUCHFILES, GLOBAL_TOUCHFILES } from './helpers/touchfiles'; // Runs when EVALS=1 is set (requires ANTHROPIC_API_KEY in env) — the EVALS
// gate lives in the shared describeIfSelected. Selection machinery is shared
const ROOT = path.resolve(import.meta.dir, '..'); // with the E2E suite; only the touchfiles table (LLM_JUDGE_TOUCHFILES, passed
// Run when EVALS=1 is set (requires ANTHROPIC_API_KEY in env) // explicitly below) differs. No EVALS_TIER filter applies here — LLM-judge
const evalsEnabled = !!process.env.EVALS; // tests have no E2E_TIERS entries and run in both tier lanes.
const describeEval = evalsEnabled ? describe : describe.skip; import {
ROOT,
computeDiffSelection,
createEvalCollector,
finalizeEvalCollector,
describeIfSelected as describeIfSelectedShared,
testConcurrentIfSelected,
} from './helpers/e2e-helpers';
// Eval result collector // Eval result collector
const evalCollector = evalsEnabled ? new EvalCollector('llm-judge') : null; const evalCollector = createEvalCollector('llm-judge');
// --- Diff-based test selection --- // --- Diff-based test selection (LLM_JUDGE_TOUCHFILES, not the E2E table) ---
let selectedTests: string[] | null = null; const selectedTests = computeDiffSelection(LLM_JUDGE_TOUCHFILES, 'LLM-judge');
if (evalsEnabled && !process.env.EVALS_ALL) { /** Wrap a describe block to skip if none of THIS FILE's tests are selected. */
const baseBranch = process.env.EVALS_BASE
|| detectBaseBranch(ROOT)
|| 'main';
const changedFiles = getChangedFiles(baseBranch, ROOT);
if (changedFiles.length > 0) {
const selection = selectTests(changedFiles, LLM_JUDGE_TOUCHFILES, GLOBAL_TOUCHFILES);
selectedTests = selection.selected;
process.stderr.write(`\nLLM-judge selection (${selection.reason}): ${selection.selected.length}/${Object.keys(LLM_JUDGE_TOUCHFILES).length} tests\n`);
if (selection.skipped.length > 0) {
process.stderr.write(` Skipped: ${selection.skipped.join(', ')}\n`);
}
process.stderr.write('\n');
}
}
/** Wrap a describe block to skip if none of its tests are selected. */
function describeIfSelected(name: string, testNames: string[], fn: () => void) { function describeIfSelected(name: string, testNames: string[], fn: () => void) {
const anySelected = selectedTests === null || testNames.some(t => selectedTests!.includes(t)); describeIfSelectedShared(name, testNames, fn, selectedTests);
(anySelected ? describeEval : describe.skip)(name, fn);
} }
/** Skip an individual test if not selected (for multi-test describe blocks). */ /** Per-test gate against this file's selection (concurrent, as before). */
function testIfSelected(testName: string, fn: () => Promise<void>, timeout: number) { function testIfSelected(testName: string, fn: () => Promise<void>, timeout: number) {
const shouldRun = selectedTests === null || selectedTests.includes(testName); testConcurrentIfSelected(testName, fn, timeout, selectedTests);
(shouldRun ? test.concurrent : test.skip)(testName, fn, timeout);
} }
describeIfSelected('LLM-as-judge quality evals', [ describeIfSelected('LLM-as-judge quality evals', [
@ -870,12 +858,4 @@ ${voiceSection}`);
}); });
// Module-level afterAll — finalize eval collector after all tests complete // Module-level afterAll — finalize eval collector after all tests complete
afterAll(async () => { afterAll(() => finalizeEvalCollector(evalCollector));
if (evalCollector) {
try {
await evalCollector.finalize();
} catch (err) {
console.error('Failed to save eval results:', err);
}
}
});