From 7db16247a9f935c4e72de9e961809e8a9e7a5c94 Mon Sep 17 00:00:00 2001 From: alfredorubin96 Date: Sun, 26 Jul 2026 14:52:06 +0200 Subject: [PATCH] fix(ci): trigger on scripts/, and actually run its 71 orphaned tests (#1263) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps, and the second was the real one. CI's path filter did not include scripts/, so a change confined to it skipped CI entirely. scripts/ holds the plugin and connector import generators that run as predev/prebuild — a break there stops every dev and build. More seriously: scripts/__tests__ was run by NO workflow at all. 71 tests, including coverage of those generators, had never gated a PR. They could not be run with one command either, because the directory mixes two runners: three files use vitest, two use node's built-in node:test. A single 'vitest run scripts/__tests__' fails on the latter with 'No test suite found'. Rather than rewrite 59 assertions across two currently-PASSING files — 11 of them multi-line, so a regex conversion would risk quiet damage — the two runners' file sets are now self-maintaining by naming: *.test.mjs for vitest, *.node-test.mjs for node --test. Adding a file to either set needs no script change. Standardising on one runner is worth doing, but not as a drive-by rewrite of green tests. Also excludes .claude/worktrees from vitest discovery: stale agent worktrees on disk were being picked up as duplicate test files. Correction to this issue's premise: I claimed 'eslint . covers scripts/'. It does not. eslint.config.js has a config block only for **/*.{ts,tsx}, so root JS matches no block and is silently unlinted. Adding coverage surfaces 33 problems across 8 files — a separate change, filed rather than smuggled in here. Adding scripts/** to the paths still matters: it is what makes the new test:scripts step run. verify now includes test:scripts: 248 + 115 + 28 + 3 vitest files, plus 36 node:test assertions. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 18 ++++++++++++++++++ package.json | 5 +++-- ...s => generate-plugin-imports.node-test.mjs} | 0 ...test.mjs => setup-enterprise.node-test.mjs} | 0 4 files changed, 21 insertions(+), 2 deletions(-) rename scripts/__tests__/{generate-plugin-imports.test.mjs => generate-plugin-imports.node-test.mjs} (100%) rename scripts/__tests__/{setup-enterprise.test.mjs => setup-enterprise.node-test.mjs} (100%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 14e6d4d3..44175765 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,6 +8,11 @@ on: - 'component/**' - 'connection/**' - 'cli/**' + # Root-level code the lint job actually covers. Without these, a change + # confined to scripts/ skipped CI entirely while still being lintable + # code — and scripts/ holds the predev/prebuild import generators (#1263). + - 'scripts/**' + - 'eslint.config.js' - 'sonar-project.properties' - 'Dockerfile' - '.github/workflows/ci.yml' @@ -20,6 +25,11 @@ on: - 'component/**' - 'connection/**' - 'cli/**' + # Root-level code the lint job actually covers. Without these, a change + # confined to scripts/ skipped CI entirely while still being lintable + # code — and scripts/ holds the predev/prebuild import generators (#1263). + - 'scripts/**' + - 'eslint.config.js' - 'sonar-project.properties' - 'Dockerfile' - '.github/workflows/ci.yml' @@ -84,6 +94,14 @@ jobs: - name: Lint all packages run: npm run lint + # scripts/__tests__ was previously run by NO workflow (#1263) — 71 tests + # covering, among other things, the plugin/connector import generators + # that run as predev/prebuild. A break there stops every dev and build. + # Runs here rather than in unit-tests to keep it off the oversubscribed + # four-package runner (#1240). + - name: Test root scripts + run: npm run test:scripts + # ── Job 1: Unit & integration tests with coverage ────────────────────────── # All three packages run in parallel within a single job. # Connection integration tests use GitHub service containers. diff --git a/package.json b/package.json index dde5d68d..baaabc55 100644 --- a/package.json +++ b/package.json @@ -35,10 +35,11 @@ "dev:enterprise": "bash scripts/setup-enterprise.sh && npm run dev", "prepare": "husky", "postinstall": "[ -n \"$CI\" ] || [ ! -d \"cli/src\" ] || (npm -w cli run build && npm link ./cli)", - "verify": "npm run typecheck && npm run lint && npm run test", + "verify": "npm run typecheck && npm run lint && npm run test && npm run test:scripts", "typecheck": "npm -w component exec tsc -- --noEmit && npm -w app exec tsc -- --noEmit", "sonar:local": "node scripts/sonar-local.mjs", - "review:local": "coderabbit review --base release/1.4 --committed -c CLAUDE.md -c .coderabbit.yaml" + "review:local": "coderabbit review --base release/1.4 --committed -c CLAUDE.md -c .coderabbit.yaml", + "test:scripts": "vitest run scripts/__tests__ --exclude '**/.claude/**' && node --test 'scripts/__tests__/*.node-test.mjs'" }, "lint-staged": { "app/**/*.{ts,tsx}": [ diff --git a/scripts/__tests__/generate-plugin-imports.test.mjs b/scripts/__tests__/generate-plugin-imports.node-test.mjs similarity index 100% rename from scripts/__tests__/generate-plugin-imports.test.mjs rename to scripts/__tests__/generate-plugin-imports.node-test.mjs diff --git a/scripts/__tests__/setup-enterprise.test.mjs b/scripts/__tests__/setup-enterprise.node-test.mjs similarity index 100% rename from scripts/__tests__/setup-enterprise.test.mjs rename to scripts/__tests__/setup-enterprise.node-test.mjs