Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/commands/test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3220,6 +3220,52 @@ function makeFailureContext(overrides: Partial<CliFailureContext> = {}): CliFail
}

describe('runFailureGet', () => {
it('--out rejects an empty path with VALIDATION_ERROR (exit 5) before any network I/O', async () => {
const { credentialsPath } = makeCreds();
let fetchCalls = 0;
const fetchImpl = makeFetch(() => {
fetchCalls += 1;
return { body: makeFailureContext() };
});
await expect(
runFailureGet(
{
profile: 'default',
output: 'text',
debug: false,
testId: 'test_failed',
failedOnly: false,
out: '',
},
{ credentialsPath, fetchImpl },
),
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
expect(fetchCalls).toBe(0);
});

it('--out rejects a path under a missing parent dir with VALIDATION_ERROR (exit 5) before any network I/O', async () => {
const { credentialsPath } = makeCreds();
let fetchCalls = 0;
const fetchImpl = makeFetch(() => {
fetchCalls += 1;
return { body: makeFailureContext() };
});
await expect(
runFailureGet(
{
profile: 'default',
output: 'text',
debug: false,
testId: 'test_failed',
failedOnly: false,
out: `/tmp/_p5_no_such_dir_${process.pid}_${Date.now()}/bundle`,
},
{ credentialsPath, fetchImpl },
),
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
expect(fetchCalls).toBe(0);
});

it('JSON mode (no --out) prints the wire envelope verbatim to stdout', async () => {
const { credentialsPath } = makeCreds();
const ctx = makeFailureContext();
Expand Down
23 changes: 14 additions & 9 deletions src/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4007,11 +4007,16 @@ export async function runFailureGet(
): Promise<FailureGetResult> {
const out = makeOutput(opts.output, deps);
const client = makeClient(opts, deps);
// We resolve the output dir BEFORE the network call so a missing /

// Resolve and validate --out BEFORE the network call so a missing /
// empty path surfaces as VALIDATION_ERROR (exit 5) without spending
// an API call. `writeBundle` re-validates internally; this is the
// fast-fail.
const requestedDir = opts.out;
// an API call. Mirrors `runArtifactGet`; `writeBundle` re-validates
// internally as defense-in-depth.
let resolvedDir: string | undefined;
if (opts.out !== undefined) {
resolvedDir = resolveBundleDir(opts.out);
await assertOutDirParentExists(resolvedDir);
}

const context = await client.get<CliFailureContext>(
`/tests/${encodeURIComponent(opts.testId)}/failure`,
Expand All @@ -4024,7 +4029,7 @@ export async function runFailureGet(
// internally; this call is the cheap upfront trap.
assertContextIntegrity(context, 'local');

if (requestedDir !== undefined) {
if (resolvedDir !== undefined) {
// Dry-run: do NOT call writeBundle (which would mkdir, fetch
// presigned URLs, and write files). Print the would-be bundle layout
// to stderr and emit the wire envelope to stdout so the agent sees
Expand All @@ -4033,18 +4038,18 @@ export async function runFailureGet(
const stderr = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`));
const fileNames = plannedBundleFiles(context, opts.failedOnly);
stderr(
`[dry-run] would write bundle to ${requestedDir} (${fileNames.length} files; meta.json renames last)`,
`[dry-run] would write bundle to ${resolvedDir} (${fileNames.length} files; meta.json renames last)`,
);
for (const f of fileNames) stderr(`[dry-run] ${f}`);
if (opts.output === 'json') {
out.print({ ok: true, dir: requestedDir, dryRun: true, context });
out.print({ ok: true, dir: resolvedDir, dryRun: true, context });
} else {
// Use a dry-run-specific renderer: the real success renderer
// says "Bundle written to ..." which would be a lie here. Stdout
// is the success contract automation may parse, so it must not
// imply the bundle was created.
out.print(
{ dir: requestedDir, files: fileNames.length, snapshotId: context.snapshotId },
{ dir: resolvedDir, files: fileNames.length, snapshotId: context.snapshotId },
data =>
renderBundleDryRunText(data as { dir: string; files: number; snapshotId: string }),
);
Expand All @@ -4053,7 +4058,7 @@ export async function runFailureGet(
}

const bundle = await writeBundle(context, {
dir: requestedDir,
dir: resolvedDir,
failedOnly: opts.failedOnly,
fetchImpl: deps.fetchImpl,
});
Expand Down