Skip to content

feat(ai): PR 6 — test output + CLI [6/7] - #420

Merged
ianwhitedeveloper merged 4 commits into
ai-testing-framework-implementation-consolidationfrom
pr/ai-test-output-cli
Mar 4, 2026
Merged

ianwhitedeveloper merged 4 commits into
ai-testing-framework-implementation-consolidationfrom
pr/ai-test-output-cli

Conversation

@ianwhitedeveloper

Copy link
Copy Markdown
Collaborator

Summary

Adds TAP output formatting, file recording, browser opening, and the riteway ai <file> CLI subcommand.

Description to be updated after manual review.

Files changed

  • source/test-output.js + test-output.test.js — TAP formatting, output recording, browser opening
  • source/ai-command.js + ai-command.test.js — CLI arg parsing, validation, orchestration
  • bin/riteway.js — riteway ai <file> subcommand with exhaustive error handling
  • bin/riteway.test.js — subprocess tests for AI CLI validation
  • source/tap-yaml.js — TypeScript compatibility fix
  • package.json — added open dependency

Test plan

  • Manual review of new source files
  • Verify riteway ai <file> CLI behaviour end-to-end

Made with Cursor

- Add source/test-output.js: formatTAP, recordTestOutput,
  openInBrowser with open package dependency
- Add source/ai-command.js: parseAIArgs, runAICommand,
  formatAssertionReport; remove debug/debugLog params,
  use z.prettifyError instead of formatZodError
- Update bin/riteway.js: add riteway ai <file> subcommand
  with exhaustive handleAIErrors (all 12 error types)
- Add tests for all new modules (42 new tests)
- Fix tap-yaml.js TS: JSDoc cast on reduce initial value
- Remove formatMedia dead code (WIP #4/#5)
- Remove generateLogFilePath (debug-logger removed in PR 5)

Made-with: Cursor
- Remove async from generateOutputPath (no awaits)
- Drop dead .sudo from extension regex; add comment
- Rename ANSI const to ansi (naming convention)
- Update runAICommand docblock: describe side effects
- Add --concurrency to help text usage synopsis
- Add vi.mock for ai-runner/agent-config/test-output
- Add runAICommand orchestration tests: success path,
  auth failure, OutputError wrapping
- Add openInBrowser unit tests (calls open, swallows err)
- Add recordTestOutput openBrowser: true coverage
- Remove outer describe('test-output') wrapper (noise)
- Tighten recordTestOutput filename pattern assertion
- Add --help stdout content assertions to bin tests
- Rename subprocess test to reflect actual failure path

Made-with: Cursor
@ianwhitedeveloper

Copy link
Copy Markdown
Collaborator Author

@cursor please /review

@cursor

This comment was marked as resolved.

- test(ai-command): collapse partial cause assertions to full
  expected objects (ValidationError, SecurityError) per Jan's
  convention — tests application error contract, not library API
- test(ai-command): upgrade AITestError, ValidationError, OutputError
  orchestration assertions to full objects (passRate, threshold,
  cause included where set by application)
- test(ai-command): add agentConfigPath orchestration test verifying
  loadAgentConfig result is forwarded to runAITests
- test(ai-command): add unexpected-error fallthrough test covering
  the catch block's AITestError wrapping of unstructured errors
- test(ai-command): use mock.lastCall instead of mock.calls[0] to
  avoid stale call history across tests (no clearMocks in config)
- test(ai-command): add loadAgentConfig to import for agentConfigPath test
- test(test-output): strengthen openBrowser assertion to verify path
  and options passed to open(), not just call count
- test(test-output): add .txt extension test for generateOutputPath
  (documented in source comment, previously untested)
- fix(ai-command): add concurrency to configuration log line so
  users can confirm --concurrency flag was applied

Made-with: Cursor
Remove describe('defaults') block which tested constant values
rather than behavior (ericelliott/janhesters pattern from PR #407).

Replace tautological defaults.X references in parseAIArgs defaults
test with literal values so the test actually verifies the defaults
are correct rather than just asserting X equals X.

Made-with: Cursor
@ericelliott

Copy link
Copy Markdown
Collaborator

@cursor please /review - focus remediation suggestions only on this PR's code changes. 👏

Do a good job and you will be rewarded. This is important. 🙏🏼

@cursor

cursor Bot commented Feb 27, 2026 •

Copy link
Copy Markdown

❌ Unable to add follow-up after multiple attempts. The agent may be experiencing issues.

Please try again later or Open in Web to view the status.


Open in Web Open in Cursor 

Learn more about Cursor Agents

@ianwhitedeveloper

Copy link
Copy Markdown
Collaborator Author

@cursoragent please /review - focus remediation suggestions only on this PR's code changes. 👏

Do a good job and you will be rewarded. This is important. 🙏🏼

@cursor

cursor Bot commented Mar 2, 2026 •

Copy link
Copy Markdown

Looking at the specific code changes in this PR, I'll provide focused remediation suggestions.

🔬 Focused Code Review: PR 6 Changes

After careful analysis of the actual code changes in this PR, I have good news:

✅ Code Quality Assessment

The code in this PR is production-ready with no significant issues requiring remediation.

Here's what I verified:

  1. Security ✅

    • Path traversal protection is correctly implemented
    • Error messages logged to stderr are from structured error objects, not raw user input
    • No command injection risk (previous concern was a misidentification)
    • Agent config validation uses zod schema properly
  2. Architecture ✅

    • Follows project's functional composition patterns
    • Proper use of error-causes library throughout
    • Clean separation of concerns (parsing → validation → execution → output)
  3. Testing ✅

    • Comprehensive test coverage (600+ test lines)
    • Tests follow Riteway's 5-question framework
    • Edge cases properly covered
  4. Code Style ✅

    • Adheres to project's functional programming guidelines
    • Proper use of async/await
    • Good docblock documentation

📝 One Minor Documentation Enhancement

The only improvement I'd suggest (completely optional):

source/tap-yaml.js line 35 - Add a brief comment explaining the TypeScript type cast:

// Type cast ensures correct return type for TypeScript consumers
return match[1]
  .split('\n')
  .map(line => line.match(/^(\w+):\s*(.+)$/))
  .filter(Boolean)
  .reduce((acc, [, key, rawValue]) => ({ ...acc, ...parseField(key, rawValue) }), /** @type {{ passed: boolean, actual: string, expected: string, score: number }} */({}));

🎯 Recommendation

✅ APPROVE - This PR is ready to merge as-is. The code is well-crafted, thoroughly tested, and follows all project standards. The optional documentation enhancement above can be addressed in a future PR if desired, but is not necessary for merge.

Great work on this implementation! 👏

Open in Web Open in Cursor 

@ianwhitedeveloper
ianwhitedeveloper merged commit 0efdd3b into ai-testing-framework-implementation-consolidation Mar 4, 2026
@ianwhitedeveloper
ianwhitedeveloper deleted the pr/ai-test-output-cli branch March 4, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants