From 02c4249632d0747553ad387ac17607bb848a9484 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 29 Apr 2026 05:37:05 +0100 Subject: [PATCH] perf: speed contract test imports --- .../channel-import-guardrails.test.ts | 18 +++++--- .../test-helpers/contracts-testkit.ts | 41 +------------------ .../test-helpers/import-side-effects.ts | 39 ++++++++++++++++++ .../test-helpers/public-artifacts.ts | 5 ++- ...ntime-import-side-effects.contract.test.ts | 2 +- test/extension-test-boundary.test.ts | 2 +- 6 files changed, 59 insertions(+), 48 deletions(-) create mode 100644 src/plugin-sdk/test-helpers/import-side-effects.ts diff --git a/src/channels/plugins/contracts/channel-import-guardrails.test.ts b/src/channels/plugins/contracts/channel-import-guardrails.test.ts index 337e9cec2fa..042f9878261 100644 --- a/src/channels/plugins/contracts/channel-import-guardrails.test.ts +++ b/src/channels/plugins/contracts/channel-import-guardrails.test.ts @@ -1,9 +1,9 @@ -import { readdirSync, readFileSync } from "node:fs"; -import { dirname, resolve } from "node:path"; +import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { basename, dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; -import { GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES } from "openclaw/plugin-sdk/plugin-test-contracts"; import { describe, expect, it } from "vitest"; import { classifyBundledExtensionSourcePath } from "../../../../scripts/lib/extension-source-classifier.mjs"; +import { GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES } from "../../../plugin-sdk/test-helpers/public-artifacts.js"; import { loadPluginManifestRegistry } from "../../../plugins/manifest-registry.js"; const ROOT_DIR = resolve(dirname(fileURLToPath(import.meta.url)), "../../.."); @@ -15,7 +15,9 @@ const bundledPluginRecords = loadPluginManifestRegistry({ config: {}, }).plugins.filter((plugin) => plugin.origin === "bundled"); const bundledPluginRoots = new Map( - bundledPluginRecords.map((plugin) => [plugin.id, plugin.rootDir] as const), + bundledPluginRecords.map( + (plugin) => [plugin.id, resolveBundledPluginSourceRoot(plugin.rootDir)] as const, + ), ); const BUNDLED_EXTENSION_IDS = [...bundledPluginRoots.keys()].toSorted( (left, right) => right.length - left.length, @@ -44,6 +46,12 @@ const GUARDED_CHANNEL_EXTENSIONS = new Set([ "zalo", "zalouser", ]); + +function resolveBundledPluginSourceRoot(rootDir: string): string { + const sourceRoot = resolve(REPO_ROOT, BUNDLED_PLUGIN_ROOT_DIR, basename(rootDir)); + return existsSync(sourceRoot) ? sourceRoot : rootDir; +} + function bundledPluginFile(pluginId: string, relativePath: string): string { const rootDir = bundledPluginRoots.get(pluginId); if (!rootDir) { @@ -343,7 +351,7 @@ function collectExtensionSourceFiles(): string[] { } extensionSourceFilesCache = bundledPluginRecords.flatMap((plugin) => collectSourceFiles(undefined, { - rootDir: plugin.rootDir, + rootDir: resolveBundledPluginSourceRoot(plugin.rootDir), shouldSkipEntry: ({ entryName, normalizedFullPath }) => classifyBundledExtensionSourcePath(normalizedFullPath).isTestLike || entryName === "api.ts" || diff --git a/src/plugin-sdk/test-helpers/contracts-testkit.ts b/src/plugin-sdk/test-helpers/contracts-testkit.ts index 5cb9a0fa5e2..75aaa70e0f4 100644 --- a/src/plugin-sdk/test-helpers/contracts-testkit.ts +++ b/src/plugin-sdk/test-helpers/contracts-testkit.ts @@ -8,50 +8,11 @@ import { type PluginRecord, type PluginRuntime, } from "../testing.js"; +export { assertNoImportTimeSideEffects } from "./import-side-effects.js"; import { uniqueSortedStrings } from "./string-utils.js"; export { registerProviders, requireProvider, uniqueSortedStrings }; -function formatImportSideEffectCall(args: readonly unknown[]): string { - if (args.length === 0) { - return "(no args)"; - } - return args - .map((arg) => { - try { - return JSON.stringify(arg); - } catch { - return String(arg); - } - }) - .join(", "); -} - -export function assertNoImportTimeSideEffects(params: { - moduleId: string; - forbiddenSeam: string; - calls: readonly (readonly unknown[])[]; - why: string; - fixHint: string; -}) { - if (params.calls.length === 0) { - return; - } - const observedCalls = params.calls - .slice(0, 3) - .map((call, index) => ` ${index + 1}. ${formatImportSideEffectCall(call)}`) - .join("\n"); - throw new Error( - [ - `[runtime contract] ${params.moduleId} touched ${params.forbiddenSeam} during module import.`, - `why this is banned: ${params.why}`, - `expected fix: ${params.fixHint}`, - `observed calls (${params.calls.length}):`, - observedCalls, - ].join("\n"), - ); -} - export function createPluginRegistryFixture(config = {} as OpenClawConfig) { return { config, diff --git a/src/plugin-sdk/test-helpers/import-side-effects.ts b/src/plugin-sdk/test-helpers/import-side-effects.ts new file mode 100644 index 00000000000..af2eb0e93e1 --- /dev/null +++ b/src/plugin-sdk/test-helpers/import-side-effects.ts @@ -0,0 +1,39 @@ +function formatImportSideEffectCall(args: readonly unknown[]): string { + if (args.length === 0) { + return "(no args)"; + } + return args + .map((arg) => { + try { + return JSON.stringify(arg); + } catch { + return String(arg); + } + }) + .join(", "); +} + +export function assertNoImportTimeSideEffects(params: { + moduleId: string; + forbiddenSeam: string; + calls: readonly (readonly unknown[])[]; + why: string; + fixHint: string; +}) { + if (params.calls.length === 0) { + return; + } + const observedCalls = params.calls + .slice(0, 3) + .map((call, index) => ` ${index + 1}. ${formatImportSideEffectCall(call)}`) + .join("\n"); + throw new Error( + [ + `[runtime contract] ${params.moduleId} touched ${params.forbiddenSeam} during module import.`, + `why this is banned: ${params.why}`, + `expected fix: ${params.fixHint}`, + `observed calls (${params.calls.length}):`, + observedCalls, + ].join("\n"), + ); +} diff --git a/src/plugin-sdk/test-helpers/public-artifacts.ts b/src/plugin-sdk/test-helpers/public-artifacts.ts index 1448b92870d..29179260ae1 100644 --- a/src/plugin-sdk/test-helpers/public-artifacts.ts +++ b/src/plugin-sdk/test-helpers/public-artifacts.ts @@ -1,4 +1,7 @@ -import { assertUniqueValues, BUNDLED_RUNTIME_SIDECAR_PATHS } from "../testing.js"; +import { + assertUniqueValues, + BUNDLED_RUNTIME_SIDECAR_PATHS, +} from "../../plugins/runtime-sidecar-paths.js"; export function getPublicArtifactBasename(relativePath: string): string { return relativePath.split("/").at(-1) ?? relativePath; diff --git a/src/plugins/contracts/runtime-import-side-effects.contract.test.ts b/src/plugins/contracts/runtime-import-side-effects.contract.test.ts index 1eae19ad2bb..1e98115e515 100644 --- a/src/plugins/contracts/runtime-import-side-effects.contract.test.ts +++ b/src/plugins/contracts/runtime-import-side-effects.contract.test.ts @@ -1,5 +1,5 @@ -import { assertNoImportTimeSideEffects } from "openclaw/plugin-sdk/plugin-test-contracts"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { assertNoImportTimeSideEffects } from "../../plugin-sdk/test-helpers/import-side-effects.js"; const listChannelPlugins = vi.hoisted(() => vi.fn(() => [ diff --git a/test/extension-test-boundary.test.ts b/test/extension-test-boundary.test.ts index 678469bbe23..4833f926973 100644 --- a/test/extension-test-boundary.test.ts +++ b/test/extension-test-boundary.test.ts @@ -1,8 +1,8 @@ import fs from "node:fs"; import path from "node:path"; -import { GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES } from "openclaw/plugin-sdk/plugin-test-contracts"; import { BUNDLED_PLUGIN_PATH_PREFIX } from "openclaw/plugin-sdk/test-fixtures"; import { describe, expect, it } from "vitest"; +import { GUARDED_EXTENSION_PUBLIC_SURFACE_BASENAMES } from "../src/plugin-sdk/test-helpers/public-artifacts.js"; const repoRoot = path.resolve(import.meta.dirname, ".."); const ALLOWED_EXTENSION_PUBLIC_SURFACE_BASENAMES = new Set(