mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-31 02:45:40 +00:00
ci(skills): run skills/**/*.test.mjs in CI (#1724)
skills/**/*.test.mjs files (e.g. skills/media-use/scripts/resolve.test.mjs and skills/media-use/scripts/lib/manifest.test.mjs) are bare `node --test` files with only `node:` built-in imports. They aren't part of any workspace package, and the existing `Test` job's path filter (the `code` filter in the `changes` job) excludes `skills/**`, so even on PRs that touch only skills/ those tests never run. This matters for regression guards. The shell-injection probe test added in HF#1723 feeds probe() a filename containing `clip"; touch INJECTED; echo ".mp4` and asserts no marker file is created. The test passes locally but under the current job graph it would never run in CI on a follow-up skills/ change that re-introduces the bug. Closing the gap with a dedicated `Test: skills` job rather than relaxing the `code` filter. The existing `Test` job's steps run `bun run test:scripts` (hardcoded file list) and `bun run --filter '*' test` (workspace packages only), neither of which would actually execute skills tests even if the filter let `skills/**` through. The dedicated job needs no `bun install`, just node 22, since the tests only import from `node:` and relative paths. The discovery step shells out to `find` and fails loudly when zero test files match, so a future rename or layout change can't silently turn this into a no-op pass. Spotted by Via in HF#1723 review thread, confirmed by James as a separate follow-up rather than a blocker for HF#1723. -- Jerrai (https://claude.com/claude-code)
This commit is contained in:
@@ -229,11 +229,16 @@ jobs:
|
||||
- run: bun run --cwd packages/core build:hyperframes-runtime
|
||||
- run: bun run --filter '!@hyperframes/producer' test
|
||||
|
||||
# Runs the skills' own node:test suites (e.g. media-use), which live outside
|
||||
# the workspace packages and are not covered by the `test` job above. Gated on
|
||||
# the `skills` filter so a skills-only PR actually exercises them in CI.
|
||||
# Tests under skills/**/*.test.mjs are bare `node --test` files with only
|
||||
# `node:` built-in imports. They aren't part of any workspace package, and
|
||||
# the main `Test` job's `code` path filter excludes `skills/**`, so without
|
||||
# this dedicated job they'd never run in CI. Examples:
|
||||
# * skills/media-use/scripts/resolve.test.mjs
|
||||
# * skills/media-use/scripts/lib/manifest.test.mjs
|
||||
# Several of these are regression guards (e.g. shell-injection cases), so
|
||||
# the whole point is that they fire on PRs that touch skills/.
|
||||
test-skills:
|
||||
name: Test (skills)
|
||||
name: "Test: skills"
|
||||
needs: changes
|
||||
if: needs.changes.outputs.skills == 'true'
|
||||
runs-on: ubuntu-latest
|
||||
@@ -243,7 +248,20 @@ jobs:
|
||||
- uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
|
||||
with:
|
||||
node-version: 22
|
||||
- run: node --test 'skills/**/*.test.mjs'
|
||||
- name: Discover and run skills tests
|
||||
# We expand the test list via bash so the job fails loudly when the
|
||||
# matcher comes back empty, rather than silently no-op'ing (which
|
||||
# would defeat the whole point of this job).
|
||||
run: |
|
||||
set -euo pipefail
|
||||
mapfile -t SKILLS_TESTS < <(find skills -type f -name "*.test.mjs" | sort)
|
||||
if [ "${#SKILLS_TESTS[@]}" -eq 0 ]; then
|
||||
echo "::error::No skills/**/*.test.mjs files found. Did the layout change?"
|
||||
exit 1
|
||||
fi
|
||||
printf 'Running %d skills test file(s):\n' "${#SKILLS_TESTS[@]}"
|
||||
printf ' * %s\n' "${SKILLS_TESTS[@]}"
|
||||
node --test "${SKILLS_TESTS[@]}"
|
||||
|
||||
cli-npx-shim:
|
||||
name: "CLI: npx shim (${{ matrix.os }})"
|
||||
|
||||
Reference in New Issue
Block a user