Skip to content

test: add HTTP request regression snapshots - #290

Draft
penalosa wants to merge 11 commits into
mainfrom
penalosa/regression-testing
Draft

penalosa wants to merge 11 commits into
mainfrom
penalosa/regression-testing

Conversation

@penalosa

@penalosa penalosa commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Adds a dedicated Vitest + MSW regression suite that runs the real cf CLI for every generated API command and compares every recorded HTTP request with persisted snapshots. It is intended to catch request changes during large CLI refactors.

The suite covers 2,954 generated commands and 1,243 command groups. Each command runs with no arguments and with a large set of argument combinations: required values, individual flags, enum and boolean values, pairwise flag presence, positional alternates, aliases, global options, local mode, body inputs, and file uploads. Snapshots record the method, full URL, headers, JSON or binary body, and ordered multipart parts. Failures name the command, snapshot, changed case, and exact arguments alongside the request diff. Every command must also reach its declared method and endpoint.

Every command listed in hand-written-commands.json is excluded, along with groups that only contain hand-written commands. Those commands orchestrate local projects, external tools such as Docker, SSH and cloudflared, and multi-request workflows, which would need fixtures and fakes outside the scope of this suite. Mock responses are empty success envelopes apart from the zone-name lookup used by --zone <domain>. Local mode keeps request rewriting real and routes Miniflare's requests through MSW.

The command cases are split into 12 files balanced by case count. The new HTTP regression workflow runs them as 12 Linux GitHub Actions matrix jobs, one file per job, using Vitest --shard. The small harness tests run with the normal CLI suite on Linux, macOS, and Windows.

Generated snapshots make up almost all of the diff and are marked as generated in .gitattributes.

Validation:

  • pnpm check
  • pnpm --filter cf test:regression — 4,197 tests passed in 96.17s locally
  • pnpm --filter cf test src/__tests__/regression/harness.test.ts

@pkg-pr-new

pkg-pr-new Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
npm i https://pkg.pr.new/cf@290

commit: 26d71fc

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

cf --help startup

mean stddev min max
PR 260.6 ms 4.4 ms 255.9 ms 272.2 ms
base 262.3 ms 4.1 ms 257.0 ms 270.1 ms

Δ (PR vs base): ↓ -1.8 ms (-0.7%)

Measured with hyperfine on ubuntu-latest (3 warmup runs, 20 timed runs). CI numbers carry ±a few ms of runner jitter; treat small deltas as noise.

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

🤖 I've done a thorough review of your PR.

Adds a parallel MSW/Vitest snapshot suite that records request behavior for the generated and hand-written CLI command surface.

  1. Warning: cf tunnels run is hand-written and bypasses the generic generated-command endpoint assertion. It performs /cfd_tunnel lookup requests, but is absent from workflowRequestPaths, so that request can silently disappear when its snapshot is updated.
"cf tunnels run": "/cfd_tunnel",
"cf tunnels tail": "/management",
  1. Warning: The workflow assertion only checks a path substring, so it accepts prerequisite reads in place of the primary mutation. Preview deploy's GET .../deployments/latest passes in place of its POST .../deployments, and image deletion's HEAD passes in place of DELETE.
const workflowPath = workflowRequestPaths[command.command];
const workflowMethod =
  command.command === "cf previews deploy"
    ? "POST"
    : command.command === "cf containers images delete"
      ? "DELETE"
      : undefined;
if (workflowPath) {
  expect(
    recordings.some((recording) =>
      recording.requests.some(
        (request) =>
          (!workflowMethod || request.method === workflowMethod) &&
          new URL(request.url).pathname.includes(workflowPath)
      )
    ),
    `${command.command} never reached ${workflowPath}. Fix its fixture before updating the request baseline.`
  ).toBe(true);
}

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

🤖 I've done a thorough review of your PR.

Adds HTTP request regression snapshots across the CLI surface.

  1. Warning: cf tunnels run lacks workflow endpoint coverage.
  2. Warning: Path-only workflow checks can accept preflight requests instead of mutations.

github run

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

The workflow request guard has two gaps that let required hand-written API calls disappear when baselines are updated.

  1. cf tunnels run is hand-written and bypasses the generated-command endpoint assertion. When no token is supplied it resolves the tunnel through GET /cfd_tunnel, but workflowRequestPaths does not require that request. Store an expected method with each path and include this flow:
export const workflowRequestPaths: Record<
	string,
	{ path: string; method: string }
> = {
	"cf ai run": { path: "/ai/run", method: "POST" },
	"cf docs search": { path: "/api/ai-search/search", method: "POST" },
	"cf registrar registrations create": {
		path: "/registrar/registrations",
		method: "POST",
	},
	"cf workers versions create": { path: "/versions", method: "POST" },
	"cf workers triggers deploy": { path: "/schedules", method: "PUT" },
	"cf previews deploy": { path: "/deployments", method: "POST" },
	"cf containers images list": { path: "/_catalog", method: "GET" },
	"cf containers images delete": { path: "/manifests/", method: "DELETE" },
	"cf d1 migrations list": { path: "/query", method: "POST" },
	"cf d1 migrations apply": { path: "/query", method: "POST" },
	"cf tunnels run": { path: "/cfd_tunnel", method: "GET" },
	"cf tunnels tail": { path: "/management", method: "POST" },
};
  1. The suite checks only a pathname. Preview deploy records GET .../deployments/latest before its required POST .../deployments, and image deletion similarly sends HEAD before DELETE; dropping either mutation still satisfies the check. Require the mapped HTTP verb:
				const workflow = workflowRequestPaths[command.command];
				if (workflow) {
					expect(
						recordings.some((recording) =>
							recording.requests.some(
								(request) =>
									request.method === workflow.method &&
									new URL(request.url).pathname.includes(workflow.path)
							)
						),
						`${command.command} never reached ${workflow.method} ${workflow.path}. Fix its fixture before updating the request baseline.`
					).toBe(true);
				}

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Review: 2 findings.

🤖 I've done a thorough review of your PR.

Adds HTTP request regression snapshots and CI coverage.

  1. Warning: cf tunnels run lacks a required API endpoint guard.
  2. Warning: workflow guards check paths but not HTTP methods, allowing prerequisite requests to mask missing mutations.

Posted both fixes as GitHub suggestion comments.

github run

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

The workflow guard needs to check the expected HTTP method as well as the path. The preview snapshot records GET .../deployments/latest before the required POST .../deployments, and image deletion records HEAD .../manifests/... before DELETE; removing a mutation and updating its snapshot would still pass.

export const workflowRequestPaths: Record<
	string,
	{ path: string; method: string }
> = {
	"cf ai run": { path: "/ai/run", method: "POST" },
	"cf docs search": { path: "/api/ai-search/search", method: "POST" },
	"cf registrar registrations create": {
		path: "/registrar/registrations",
		method: "POST",
	},
	"cf workers versions create": { path: "/versions", method: "POST" },
	"cf workers triggers deploy": { path: "/schedules", method: "PUT" },
	"cf previews deploy": { path: "/deployments", method: "POST" },
	"cf containers images list": { path: "/_catalog", method: "GET" },
	"cf containers images delete": { path: "/manifests/", method: "DELETE" },
	"cf d1 migrations list": { path: "/query", method: "POST" },
	"cf d1 migrations apply": { path: "/query", method: "POST" },
};
const workflow = workflowRequestPaths[command.command];
if (workflow) {
	expect(
		recordings.some((recording) =>
			recording.requests.some(
				(request) =>
					request.method === workflow.method &&
					new URL(request.url).pathname.includes(workflow.path)
			)
		),
		`${command.command} never reached ${workflow.method} ${workflow.path}. Fix its fixture before updating the request baseline.`
	).toBe(true);
}

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

🤖 I've done a thorough review of your PR.

Adds parallel MSW request snapshots and Linux CI coverage.

  1. Warning: Workflow guards accept prerequisite reads instead of required mutations. Posted a suggestion comment: test: add HTTP request regression snapshots #290 (comment)

github run

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

No new concerns since the last review

github run

@ask-bonk ask-bonk Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suite claims alias coverage, but every included invocation uses canonical option names. In particular, cf containers images delete is exercised with --force but never its public -f alias, so a regression in that yargs alias or telemetry alias mapping can be accepted by updating this snapshot. Generate one equivalent case for each OptionMeta.alias (all alias forms are represented, and the current included surface has -f).

Comment on lines +479 to +488
const canonicalCases = [...cases];
for (const alias of command.aliases ?? []) {
for (const scenario of canonicalCases) {
cases.push({
name: `alias ${alias}: ${scenario.name}`,
args: scenario.args,
commandPath: [...command.fullPath.slice(0, -1), alias],
});
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const canonicalCases = [...cases];
for (const alias of command.aliases ?? []) {
for (const scenario of canonicalCases) {
cases.push({
name: `alias ${alias}: ${scenario.name}`,
args: scenario.args,
commandPath: [...command.fullPath.slice(0, -1), alias],
});
}
}
const canonicalCases = [...cases];
for (const option of options) {
const aliases =
typeof option.alias === "string"
? [option.alias]
: (option.alias ?? []);
for (const alias of aliases) {
const args = withOptions(matrixBase, [option], source);
const optionIndex = args.findIndex(
(arg) =>
arg === `--${option.name}` ||
arg.startsWith(`--${option.name}=`)
);
if (optionIndex >= 0) {
const flag = alias.length === 1 ? `-${alias}` : `--${alias}`;
args[optionIndex] = flag;
cases.push({ name: `alias ${flag}`, args });
}
}
}
for (const alias of command.aliases ?? []) {
for (const scenario of canonicalCases) {
cases.push({
name: `alias ${alias}: ${scenario.name}`,
args: scenario.args,
commandPath: [...command.fullPath.slice(0, -1), alias],
});
}
}

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

🤖 I've done a thorough review of your PR.

Adds HTTP request regression snapshots and CI coverage.

  1. Warning: option aliases are not exercised. Posted a suggestion for alias cases, including cf containers images delete -f.

github run

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

No new concerns since the last review

github run

@ask-bonk

ask-bonk Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@petebacondarwin
petebacondarwin requested review from a team and NuroDev and removed request for a team October 23, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant