From f5305afcfb417416b35d7d968f0849e8d07b5bac Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 20 Apr 2026 20:52:18 +0100 Subject: [PATCH] test: speed changed lanes and channel contracts --- .github/workflows/ci.yml | 4 + docs/ci.md | 5 +- scripts/lib/channel-contract-test-plan.mjs | 42 ++++++++-- src/auto-reply/reply/commands-approve.test.ts | 14 ++++ src/scripts/test-projects.test.ts | 80 +++++++++++++++++++ 5 files changed, 138 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f63ec92f86c..82f3ada4407 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -736,6 +736,10 @@ jobs: env: SHARD_RESULT: ${{ needs.checks-fast-channel-contracts-shard.result }} run: | + if [ "$SHARD_RESULT" = "cancelled" ]; then + echo "Channel contract shards were cancelled, usually because a newer commit superseded this run." >&2 + exit 1 + fi if [ "$SHARD_RESULT" != "success" ]; then echo "Channel contract shards failed: $SHARD_RESULT" >&2 exit 1 diff --git a/docs/ci.md b/docs/ci.md index f537a71dbce..d8fdf5b378a 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -51,7 +51,9 @@ Local changed-lane logic lives in `scripts/changed-lanes.mjs` and is executed by On pushes, the `checks` matrix adds the push-only `compat-node22` lane. On pull requests, that lane is skipped and the matrix stays focused on the normal test/channel lanes. -The slowest Node test families are split into include-file shards so each job stays small: channel contracts split registry and core coverage into eight balanced shards each, auto-reply reply command tests split into four include-pattern shards, and the other large auto-reply reply prefix groups split into two shards each. `check-additional` also separates package-boundary compile/canary work from runtime topology gateway/architecture work. +The slowest Node test families are split into include-file shards so each job stays small: channel contracts split registry and core coverage into eight weighted shards each, auto-reply reply command tests split into four include-pattern shards, and the other large auto-reply reply prefix groups split into two shards each. `check-additional` also separates package-boundary compile/canary work from runtime topology gateway/architecture work. + +GitHub may mark superseded jobs as `cancelled` when a newer push lands on the same PR or `main` ref. Treat that as CI noise unless the newest run for the same ref is also failing. The aggregate shard checks call out this cancellation case explicitly so it is easier to distinguish from a test failure. ## Runners @@ -74,6 +76,7 @@ pnpm check:architecture pnpm test:gateway:watch-regression pnpm test # vitest tests pnpm test:channels +pnpm test:contracts:channels pnpm check:docs # docs format + lint + broken links pnpm build # build dist when CI artifact/build-smoke lanes matter ``` diff --git a/scripts/lib/channel-contract-test-plan.mjs b/scripts/lib/channel-contract-test-plan.mjs index 6cb423a58e7..cab5ca29935 100644 --- a/scripts/lib/channel-contract-test-plan.mjs +++ b/scripts/lib/channel-contract-test-plan.mjs @@ -12,6 +12,22 @@ function listContractTestFiles(rootDir = "src/channels/plugins/contracts") { .toSorted((a, b) => a.localeCompare(b)); } +const CONTRACT_FILE_WEIGHTS = new Map([ + ["channel-import-guardrails.test.ts", 18], + ["directory.registry-backed.contract.test.ts", 12], + ["outbound-payload.contract.test.ts", 18], + ["plugin.registry-backed.contract.test.ts", 34], + ["plugins-core.catalog.paths.contract.test.ts", 28], + ["plugins-core.catalog.entries.contract.test.ts", 16], + ["session-binding.registry-backed.contract.test.ts", 16], + ["surfaces-only.registry-backed.contract.test.ts", 36], +]); + +function resolveContractFileWeight(file) { + const name = file.replaceAll("\\", "/").split("/").pop(); + return CONTRACT_FILE_WEIGHTS.get(name) ?? 8; +} + export function createChannelContractTestShards() { const rootDir = "src/channels/plugins/contracts"; const suffixes = ["a", "b", "c", "d", "e", "f", "g", "h"]; @@ -24,18 +40,32 @@ export function createChannelContractTestShards() { core: suffixes.map((suffix) => `checks-fast-contracts-channels-core-${suffix}`), registry: suffixes.map((suffix) => `checks-fast-contracts-channels-registry-${suffix}`), }; + const weights = Object.fromEntries(Object.keys(groups).map((key) => [key, 0])); const pushBalanced = (keys, file) => { - const target = keys.toSorted((a, b) => groups[a].length - groups[b].length)[0]; + const target = keys.toSorted((a, b) => weights[a] - weights[b] || a.localeCompare(b))[0]; groups[target].push(file); + weights[target] += resolveContractFileWeight(file); }; + const coreFiles = []; + const registryFiles = []; for (const file of listContractTestFiles(rootDir)) { const name = relative(rootDir, file).replaceAll("\\", "/"); - if (name.startsWith("plugins-core.") || name.startsWith("plugin.")) { - pushBalanced(groupKeys.core, file); - } else { - pushBalanced(groupKeys.registry, file); - } + (name.startsWith("plugins-core.") || name.startsWith("plugin.") + ? coreFiles + : registryFiles + ).push(file); + } + + const byDescendingWeight = (left, right) => { + const delta = resolveContractFileWeight(right) - resolveContractFileWeight(left); + return delta === 0 ? left.localeCompare(right) : delta; + }; + for (const file of registryFiles.toSorted(byDescendingWeight)) { + pushBalanced(groupKeys.registry, file); + } + for (const file of coreFiles.toSorted(byDescendingWeight)) { + pushBalanced(groupKeys.core, file); } return Object.entries(groups).map(([checkName, includePatterns]) => ({ diff --git a/src/auto-reply/reply/commands-approve.test.ts b/src/auto-reply/reply/commands-approve.test.ts index e219d5c49a7..bd84b4ff3a0 100644 --- a/src/auto-reply/reply/commands-approve.test.ts +++ b/src/auto-reply/reply/commands-approve.test.ts @@ -100,6 +100,19 @@ const slackApproveTestPlugin: ChannelPlugin = { }), }; +const whatsappApproveTestPlugin: ChannelPlugin = { + ...createChannelTestPluginBase({ + id: "whatsapp", + label: "WhatsApp", + docsPath: "/channels/whatsapp", + capabilities: { + chatTypes: ["direct", "group"], + media: true, + nativeCommands: true, + }, + }), +}; + const signalApproveTestPlugin: ChannelPlugin = { ...createChannelTestPluginBase({ id: "signal", @@ -346,6 +359,7 @@ function setApprovePluginRegistry(): void { createTestRegistry([ { pluginId: "discord", plugin: discordApproveTestPlugin, source: "test" }, { pluginId: "slack", plugin: slackApproveTestPlugin, source: "test" }, + { pluginId: "whatsapp", plugin: whatsappApproveTestPlugin, source: "test" }, { pluginId: "signal", plugin: signalApproveTestPlugin, source: "test" }, { pluginId: "telegram", plugin: telegramApproveTestPlugin, source: "test" }, ]), diff --git a/src/scripts/test-projects.test.ts b/src/scripts/test-projects.test.ts index c24f9ab3351..960dd41f3c2 100644 --- a/src/scripts/test-projects.test.ts +++ b/src/scripts/test-projects.test.ts @@ -7,6 +7,8 @@ const { buildVitestRunPlans, createVitestRunSpecs, parseTestProjectsArgs, + resolveChangedTargetArgs, + resolveChangedTestTargetPlan, resolveParallelFullSuiteConcurrency, } = (await import("../../scripts/test-projects.test-support.mjs")) as unknown as { applyParallelVitestCachePaths: ( @@ -35,6 +37,7 @@ const { buildVitestRunPlans: ( args: string[], cwd?: string, + listChangedPaths?: (baseRef: string, cwd: string) => string[], ) => Array<{ config: string; forwardedArgs: string[]; @@ -64,6 +67,17 @@ const { targetArgs: string[]; watchMode: boolean; }; + resolveChangedTargetArgs: ( + args: string[], + cwd?: string, + listChangedPaths?: (baseRef: string, cwd: string) => string[], + ) => string[] | null; + resolveChangedTestTargetPlan: ( + changedPaths: string[], + ) => + | { mode: "none"; targets: string[] } + | { mode: "targets"; targets: string[] } + | { mode: "broad"; targets: string[] }; resolveParallelFullSuiteConcurrency: ( specCount: number, env?: NodeJS.ProcessEnv, @@ -853,6 +867,72 @@ describe("test-projects args", () => { ]); }); + it("keeps docs-only changed runs empty instead of widening to the full suite", () => { + const changedPaths = ["docs/help/testing.md", "AGENTS.md"]; + + expect(resolveChangedTestTargetPlan(changedPaths)).toEqual({ + mode: "targets", + targets: [], + }); + expect( + resolveChangedTargetArgs(["--changed=origin/main"], process.cwd(), () => changedPaths), + ).toEqual([]); + expect( + buildVitestRunPlans(["--changed=origin/main"], process.cwd(), () => changedPaths), + ).toEqual([]); + }); + + it("keeps core test-only changes on their owning test lane", () => { + const changedPaths = ["src/auto-reply/reply/commands-approve.test.ts"]; + + expect( + buildVitestRunPlans(["--changed=origin/main"], process.cwd(), () => changedPaths), + ).toEqual([ + { + config: "test/vitest/vitest.auto-reply.config.ts", + forwardedArgs: [], + includePatterns: ["src/auto-reply/reply/commands-approve.test.ts"], + watchMode: false, + }, + ]); + }); + + it("widens extension-facing core contract changes to extension tests", () => { + const changedPaths = ["src/plugin-sdk/core.ts"]; + const plans = buildVitestRunPlans(["--changed=origin/main"], process.cwd(), () => changedPaths); + + expect( + resolveChangedTargetArgs(["--changed=origin/main"], process.cwd(), () => changedPaths), + ).toEqual(["src/plugin-sdk/core.ts", "extensions"]); + expect(plans[0]).toEqual({ + config: "test/vitest/vitest.plugin-sdk.config.ts", + forwardedArgs: [], + includePatterns: ["src/plugin-sdk/**/*.test.ts"], + watchMode: false, + }); + expect(plans.map((plan) => plan.config)).toContain( + "test/vitest/vitest.extension-discord.config.ts", + ); + expect(plans.map((plan) => plan.config)).toContain( + "test/vitest/vitest.extension-providers.config.ts", + ); + }); + + it("keeps extension production changes on the owning extension lane", () => { + const changedPaths = ["extensions/discord/src/monitor/message-handler.ts"]; + + expect( + buildVitestRunPlans(["--changed=origin/main"], process.cwd(), () => changedPaths), + ).toEqual([ + { + config: "test/vitest/vitest.extension-discord.config.ts", + forwardedArgs: [], + includePatterns: ["extensions/discord/src/monitor/**/*.test.ts"], + watchMode: false, + }, + ]); + }); + it("splits mixed core and extension targets into separate vitest runs", () => { expect( buildVitestRunPlans([