Conversation
27 tests across 5 suites verifying the TUI harness against the real AgentCore CLI: harness self-tests, navigation flows, create wizard, add-resource flows, and deploy screen rendering. Tests use describe.skipIf(!isAvailable) to gracefully skip when node-pty is not installed. createMinimalProjectDir provides fast (~10ms) project directory setup without npm install.
Hweinstock
left a comment
Hweinstock
left a comment
There was a problem hiding this comment.
Awesome! Two small questions, but I don't think either are blocking.
integ-tests/tui/add-flow.test.ts
Outdated
|
|
||
| // Type 'add' to filter the command list, then press Enter to select it. | ||
| await session.sendKeys('add'); | ||
| await session.waitFor('add', 3000); |
There was a problem hiding this comment.
nit: 3 seconds feels like a long timeout to load the menu. Is it worth dropping this a bit to try to catch performance regressions, or do you think we risk making the tests flaky?
| import { LaunchError, TuiSession, WaitForTimeoutError, isAvailable } from '../../src/tui-harness/index.js'; | ||
| import { afterEach, describe, expect, it } from 'vitest'; | ||
|
|
||
| describe.skipIf(!isAvailable)('TuiSession harness self-tests', () => { |
There was a problem hiding this comment.
nice, I like the idea of separating these so we can know if a regression is in the framework or the source code itself.
| try { | ||
| const { isAvailable, unavailableReason } = await import('../../src/tui-harness/lib/availability.js'); | ||
| if (!isAvailable) { | ||
| console.warn(`TUI harness unavailable: ${unavailableReason}. Skipping all TUI tests.`); |
There was a problem hiding this comment.
is there a risk that our CI could be "green" but actually silently skipping? Is it worth failing if we can't run the tests?
- Drop waitFor timeout for menu filter from 3s to 1.5s to catch performance regressions (filter echo should be near-instant) - Fail hard in CI when TUI harness is unavailable instead of silently skipping, preventing false-green CI runs - Bump create wizard completion timeout from 30s to 60s (CDK scaffolding regularly takes >30s under load)
notgitika
left a comment
notgitika
left a comment
There was a problem hiding this comment.
left 2 nits otherwise lgtm
| // (a) HomeScreen renders when no project exists | ||
| // --------------------------------------------------------------------------- | ||
| it('renders HomeScreen when launched in a directory without a project', async () => { | ||
| const bareDir = await mkdtemp(join(tmpdir(), 'tui-nav-bare-')); |
There was a problem hiding this comment.
can we do realpathSync throughout this file as well like you've done in create-flow.test.ts?
| // --------------------------------------------------------------------------- | ||
| it('navigates from HomeScreen to CreateScreen on Enter', async () => { | ||
| const bareDir = await mkdtemp(join(tmpdir(), 'tui-nav-create-')); | ||
| cleanup = () => rm(bareDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
can we also add a .catch(() => {}) in this file as well? like you have with create-flow.test.ts
3 participants
Summary
describe.skipIf(!isAvailable)to gracefully skip when node-pty is not installedTest plan
npm run test:tui— all 27 tests pass (5 suites, 0 failures)