Repository navigation
ci: run every cross-version wire test, picked up by folder so new ones can't be skipped #24499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ce7e518
7bb4271
b0a5f0c
2bf5907
8d48e8f
88e0a23
1e5e9a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| import { globSync, readFileSync } from 'node:fs' | ||
| import { parse } from 'yaml' | ||
| import { defaultExclude } from 'vitest/config' | ||
| import { describe, expect, it } from 'vitest' | ||
| import { CROSS_VERSION_WIRE_DIR, UNIT_INCLUDE, discoverUnitFiles } from './ci-unit-files.mjs' | ||
| import { classifyPrJobs } from './pr-code-change-scope.mjs' | ||
|
|
||
| function posixPaths(files) { | ||
| return files.map((file) => file.replaceAll('\\', '/')) | ||
| } | ||
|
|
||
| function workflowJobs(path) { | ||
| return parse(readFileSync(path, 'utf8')).jobs | ||
| } | ||
|
|
||
| // Why verify.needs: a job left out of it (e2e, ime, wsl) can be red or skipped without blocking a merge. | ||
| // Each gating job carries the code_paths output that decides whether it runs; a called workflow's jobs inherit the caller's. | ||
| function gatingJobs() { | ||
| const jobs = workflowJobs('.github/workflows/pr.yml') | ||
| return jobs.verify.needs.flatMap((name) => { | ||
| const { if: condition = '', uses } = jobs[name] | ||
| const gate = /needs\.code_paths\.outputs\.([\w-]+) == 'true'/.exec(condition)?.[1] | ||
| const called = uses?.startsWith('./.github/workflows/') | ||
| ? Object.values(workflowJobs(uses.slice(2))) | ||
| : [jobs[name]] | ||
| return called.map((job) => ({ gate, pathArguments: vitestPathArguments(job) })) | ||
| }) | ||
| } | ||
|
|
||
| // Path arguments of every step in the job that runs vitest; a trailing slash means a directory. | ||
| function vitestPathArguments(job) { | ||
| return (job.steps ?? []) | ||
| .map((step) => step.run) | ||
| .filter((run) => typeof run === 'string' && /\bvitest\b/.test(run)) | ||
| .flatMap((run) => run.replaceAll('\\\n', ' ').split(/\s+/)) | ||
| .map((token) => token.replace(/^['"]|['"]$/g, '')) | ||
| .filter((token) => /^(?:src|config|tests|mobile)\//.test(token)) | ||
| } | ||
|
|
||
| function runByVitestStep(file, pathArguments) { | ||
| return pathArguments.some( | ||
| (path) => path === file || (path.endsWith('/') && file.startsWith(path)) | ||
| ) | ||
| } | ||
|
|
||
| // Why module scope: globbing the unit tree costs seconds, and every test reads the same result. | ||
| const UNIT_FILES = posixPaths(globSync(UNIT_INCLUDE, { exclude: defaultExclude })) | ||
| const SHARDED_UNIT_FILES = new Set(discoverUnitFiles()) | ||
| const EXCLUDED_UNIT_FILES = UNIT_FILES.filter((file) => !SHARDED_UNIT_FILES.has(file)) | ||
| const GATING_JOBS = gatingJobs() | ||
|
|
||
| describe('unit files kept out of the sharded test job', () => { | ||
| it('each still runs in a job that gates the PR', () => { | ||
| expect(EXCLUDED_UNIT_FILES.length).toBeGreaterThan(0) | ||
| // Why: an excluded file no gating step names guards nothing, and nothing else reports it. | ||
| expect( | ||
| EXCLUDED_UNIT_FILES.filter( | ||
| (file) => !GATING_JOBS.some((job) => runByVitestStep(file, job.pathArguments)) | ||
| ) | ||
| ).toEqual([]) | ||
| }) | ||
|
|
||
| it('a change to each runs a gating job that names it', () => { | ||
| // Why: otherwise a PR editing only the file skips the one job that runs it. | ||
| expect( | ||
| EXCLUDED_UNIT_FILES.filter( | ||
| (file) => | ||
| !GATING_JOBS.some( | ||
| ({ gate, pathArguments }) => | ||
| runByVitestStep(file, pathArguments) && (!gate || classifyPrJobs([file])[gate]) | ||
| ) | ||
| ) | ||
| ).toEqual([]) | ||
| }) | ||
|
|
||
| it('runs the whole cross-version-wire directory, not a list of its files', () => { | ||
| expect(GATING_JOBS.flatMap((job) => job.pathArguments)).toContain(CROSS_VERSION_WIRE_DIR) | ||
| }) | ||
|
Comment on lines
+52
to
+78
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,105p' config/scripts/ci-unit-exclusion-coverage.test.mjs
sed -n '1,95p' config/scripts/ci-unit-files.mjsRepository: stablyai/orca Length of output: 4838 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- workflow files ---'
find .github/workflows -maxdepth 1 -type f -print | sort
printf '%s\n' '--- pr.yml ---'
cat -n .github/workflows/pr.yml
printf '%s\n' '--- locally called workflows referenced by pr.yml ---'
rg -n -C 3 'uses:\s*\./\.github/workflows/' .github/workflows/pr.yml
for f in $(rg -o 'uses:\s*\./\.github/workflows/[^[:space:]]+' .github/workflows/pr.yml | sed 's/^uses:[[:space:]]*//' | sort -u); do
printf '%s\n' "--- ${f#./} ---"
cat -n "${f#./}"
done
printf '%s\n' '--- vitest invocations and conditions across workflows ---'
rg -n -C 4 'vitest|^\s*(if|paths|paths-ignore|on):' .github/workflowsRepository: stablyai/orca Length of output: 45665 🏁 Script executed: set -eu
find .github/workflows -maxdepth 1 -type f -print | sort
printf '%s\n' '--- pr.yml ---'
cat -n .github/workflows/pr.yml
printf '%s\n' '--- locally called workflows ---'
rg -n -o 'uses:\s*\./\.github/workflows/[^\s]+' .github/workflows/pr.yml | sed 's/.*uses:[[:space:]]*//' | sort -u | while read -r path; do
printf '%s\n' "--- ${path#./} ---"
cat -n "${path#./}"
done
printf '%s\n' '--- workflow Vitest and condition context ---'
rg -n -C 4 'vitest|^\s*(if|paths|paths-ignore|on):' .github/workflowsRepository: stablyai/orca Length of output: 45665 🏁 Script executed: set -eu
find .github/workflows -maxdepth 1 -type f -print | sort
cat -n .github/workflows/pr.yml
rg -n -C 5 'uses:\s*\./\.github/workflows/|vitest|^\s*(if|paths|paths-ignore|on):' .github/workflowsRepository: stablyai/orca Length of output: 45595 🏁 Script executed: set -eu
printf '%s\n' '--- pr.yml relevant structure ---'
rg -n -C 3 'uses: \./\.github/workflows/|vitest|^\s{2}[A-Za-z0-9_.-]+:|^\s{4}if:|^\s{6}- name:' .github/workflows/pr.yml
printf '%s\n' '--- exact local workflow references ---'
rg -n 'uses: \./\.github/workflows/' .github/workflows/pr.yml
printf '%s\n' '--- Vitest commands in pr.yml and likely called workflows ---'
for f in .github/workflows/pr.yml .github/workflows/unit-tests.yml .github/workflows/unit-plan.yml .github/workflows/unit-selection-evidence.yml .github/workflows/pr-test-loc.yml; do
if test -f "$f"; then
printf '%s\n' "--- $f ---"
rg -n -C 8 'vitest|^\s{4}if:|^\s{2}[A-Za-z0-9_.-]+:|uses: \./\.github/workflows/' "$f"
fi
doneRepository: stablyai/orca Length of output: 42578 🏁 Script executed: set -eu
rg -n -C 3 'uses: \./\.github/workflows/|vitest|^\s{4}if:|^\s{2}[A-Za-z0-9_.-]+:' .github/workflows/pr.yml
rg -n 'uses: \./\.github/workflows/' .github/workflows/pr.yml
for f in .github/workflows/pr.yml .github/workflows/unit-tests.yml .github/workflows/unit-plan.yml .github/workflows/unit-selection-evidence.yml .github/workflows/pr-test-loc.yml; do
if test -f "$f"; then
printf '%s\n' "--- $f ---"
rg -n -C 8 'vitest|^\s{4}if:|^\s{2}[A-Za-z0-9_.-]+:|uses: \./\.github/workflows/' "$f"
fi
doneRepository: stablyai/orca Length of output: 44429 🏁 Script executed: set -eu
printf '%s\n' '--- classifier outline and relevant rules ---'
rg -n -C 8 'shell_contracts|cross-version-wire|test|package|git_compatibility|function|export|return|src/main/daemon|node-pty|live-shell|shell-ready' config/scripts/pr-code-change-scope.mjs
printf '%s\n' '--- complete current exclusion and discovery declarations ---'
cat -n config/scripts/ci-unit-files.mjs
printf '%s\n' '--- current diff for the changed assertion and related CI files ---'
git diff --stat 757736628f0dc0dfa972350b22524df0ca9d6f62d5 7bb42714a20e5e1c7a5264e4f140d9076a8e62d5 -- config/scripts/ci-unit-exclusion-coverage.test.mjs config/scripts/ci-unit-files.mjs .github/workflows/pr.yml config/scripts/pr-code-change-scope.mjsRepository: stablyai/orca Length of output: 22555 🏁 Script executed: set -eu
rg -n -C 8 'shell_contracts|cross-version-wire|test|package|git_compatibility|src/main/daemon|node-pty|live-shell|shell-ready' config/scripts/pr-code-change-scope.mjs
cat -n config/scripts/ci-unit-files.mjs
git diff --stat 757736628f0dc0dfa972350b22524df0ca9d6f62d5 7bb42714a20e5e1c7a5264e4f140d9076a8e62d5 -- config/scripts/ci-unit-exclusion-coverage.test.mjs config/scripts/ci-unit-files.mjs .github/workflows/pr.yml config/scripts/pr-code-change-scope.mjsRepository: stablyai/orca Length of output: 17294 🏁 Script executed: set -eu
target='src/main/runtime/structured-session-cli-login-shell.live-shell.test.ts'
printf '%s\n' '--- exact path references in PR workflows and configs ---'
rg -n -C 4 -F "$target" .github/workflows config/scripts config/vitest.config.ts package.json
printf '%s\n' '--- Vitest config and unit exclusion references ---'
rg -n -C 8 'UNIT_EXCLUDE|unit-files|defaultExclude|exclude:|include:|structured-session|live-shell|vitest' config/vitest.config.ts config/scripts package.json
printf '%s\n' '--- classifier shell detector and changed-path behavior ---'
sed -n '80,110p' config/scripts/pr-code-change-scope.mjs
sed -n '418,472p' config/scripts/pr-code-change-scope.mjsRepository: stablyai/orca Length of output: 45663 🏁 Script executed: set -eu
target='src/main/runtime/structured-session-cli-login-shell.live-shell.test.ts'
rg -n -C 4 -F "$target" .github/workflows config/scripts config/vitest.config.ts package.json
rg -n -C 8 'UNIT_EXCLUDE|unit-files|defaultExclude|exclude:|include:|structured-session|live-shell|vitest' config/vitest.config.ts config/scripts package.json
sed -n '80,110p' config/scripts/pr-code-change-scope.mjs
sed -n '418,472p' config/scripts/pr-code-change-scope.mjsRepository: stablyai/orca Length of output: 45655 Route the structured-session shell test to
Suggested fix 'src/main/terminal-history-fish',
'src/main/zsh-',
+ 'src/main/runtime/structured-session-cli-login-shell',
'src/renderer/src/components/terminal-pane/fish-color-scheme', |
||
|
|
||
| it('names every cross-version-wire test so the unit include picks it up', () => { | ||
| // Why: a directory argument only reaches files the vitest include matches; any other name is silently skipped. | ||
| const tests = posixPaths( | ||
| globSync(`${CROSS_VERSION_WIRE_DIR}**/*.{test,spec}.{js,cjs,mjs,ts,tsx}`) | ||
| ) | ||
| const included = new Set(UNIT_FILES) | ||
| expect(tests.length).toBeGreaterThan(0) | ||
| expect(tests.filter((file) => !included.has(file))).toEqual([]) | ||
| }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This treats a directory argument as a root-anchored prefix (
file.startsWith(path)), but Vitest matches positional arguments as substrings — confirmed in this repo:vitest list --config config/vitest.config.ts cross-version-wire/selects the same files, and thepr.ymlcomment at the mobile-web-app step documents the substring semantics. A future workflow edit using a shorter but valid filter such ascross-version-wire/would run the tests yet fail this guard. Matching withfile.includes(path)mirrors Vitest; if the stricter root-anchored contract is intentional, a one-line comment would make that explicit.