Fix example runtime test lanes - #73
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe PR splits the root test command into package, script, example, and full lanes, adds a script that discovers and runs example runtime tests, updates CI and FAS config, and adds new FAS tracker entries and task documents for igniteTools work. ChangesExample Runtime Test Lane Split
FAS Task Planning Documents
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## beta #73 +/- ##
=======================================
Coverage ? 89.96%
=======================================
Files ? 32
Lines ? 1515
Branches ? 435
=======================================
Hits ? 1363
Misses ? 142
Partials ? 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.fas/TASKS.md:
- Around line 1634-1643: The task tracker contains duplicate Phase A/Phase B
rows for the same briefs/slugs, which creates conflicting state in FAS. Update
the existing task entries in the TASKS.md tracker in place or remove the
duplicate rows for the igniteTools showcase Phase A and Phase B items, using the
task title/brief/slugs to identify the records that already exist above.
In @.fas/tasks/build-a-headless-anthropic-tool-use-dogfood-example-for-igni.md:
- Line 1: The task title is truncated and ends at “pluggable-”, so complete the
heading text in the markdown task file so it reads as a full searchable title.
Update the existing task title only, keeping the same intent and using the same
task heading so it is no longer cut off.
In `@scripts/test-examples.mjs`:
- Around line 104-107: The example discovery flow in test-examples.mjs is
silently filtering out test files whose parent chain does not resolve to a valid
example root, which can hide orphaned runtime tests. Update the
findTestFiles/findExampleRoot pipeline so these orphaned .test.* files are
detected before building exampleRoots and cause a hard failure with a clear
error, instead of being dropped by the Boolean filter; keep the validation close
to the exampleRoots construction and the findExampleRoot mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab294c1f-9304-4123-851c-97a30954d968
📒 Files selected for processing (15)
.fas-config.json.fas/TASKS.md.fas/tasks/add-an-observe-observation-channel-to-the-neutral-ignitetool.md.fas/tasks/build-a-headless-anthropic-tool-use-dogfood-example-for-igni.md.fas/tasks/fix-example-runtime-test-lanes-before-next-ignitetools-task.md.fas/tasks/ignitetools-pr2-coderabbit-follow-ups-bind-runtime-execute-s.md.fas/tasks/ignitetools-showcase-phase-a-make-the-headless-agent-runtime.md.fas/tasks/ignitetools-showcase-phase-b-headless-smart-home-agent-examp.md.fas/tasks/ignitetools-showcase-phase-c-terminal-to-browser-bridge-head.md.fas/tasks/implement-ignitetools-anthropic-tooldialect-adapter-at-ignit.md.fas/tasks/split-the-agent-runtime-host-type-eventtarget-from-the-rende.md.github/workflows/ci.ymlpackage.jsonscripts/__tests__/test-examples.test.mjsscripts/test-examples.mjs
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Read the remaining CodeRabbit pre-merge docstring coverage warning. Leaving it as non-blocking for this PR: this repo does not have a docstring coverage gate, the changed runner follows the existing script style, and local/FAS verification plus CI pass. The three actionable inline findings were fixed in d3b52a9 and resolved. |
Summary
Fixes the Phase B closeout test-lane shape by replacing the one-off smart-home Vitest command in the root
testscript with explicit package, script, example-runtime, and full test lanes.What changed
scripts/test-examples.mjsto discover top-level example roots with runtime tests and run Vitest against each example's own Vite/Vitest config without making examples pnpm workspace members.scripts/__tests__/test-examples.test.mjsto guard the discovery list for current runtime-tested examples.test:packages,test:scripts,test:examples, andtest:full; roottestnow runs package tests only.testCommandatnpm run test:fullso FAS verification covers package tests plus example runtime tests.Why
PR #71 tactically made root
npm run testrun only the smart-home example test. That caught the changed example test but left the long-term test organization wrong because other top-level examples already have runtime tests and examples intentionally remain outsidepnpm-workspace.yaml.Validation
npm run test:examplesnpm run test:fullfas validate-task.fas/scripts/verify.sh --fullfas commit-plan checkFinal full verification passed before push. The branch push hook also ran package tests successfully.
Summary by CodeRabbit
Bug Fixes
Tests
test:packages,test:scripts,test:examples, andtest:full.