Skip to content

test(tui): cover externalCli launchHermesCommand spawn shape - #22296

Closed
wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:tests/tui-external-cli
Closed

test(tui): cover externalCli launchHermesCommand spawn shape#22296
wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:tests/tui-external-cli

Conversation

@wesleysimplicio

@wesleysimplicio wesleysimplicio commented May 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

test(tui): cover externalCli launchHermesCommand spawn shape

Root cause

The detailed rationale from the original PR body is preserved below. This template update keeps the review structure consistent with #29640.

Fix

Why this shape

This shape mirrors #29640 so reviewers can quickly compare scope, root cause, fix, tests, and related context without having to decode a custom PR description.

Tests

  • Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.
Original body

Related PRs / issues

  • Original body preserved below for full context.
Original body

Summary

test(tui): cover externalCli launchHermesCommand spawn shape

What Changed

  • Standardized this PR body to the current Hermes Turbo template.
  • Preserved the original detailed description below for reference.

Fluxo

A mudança continua seguindo o fluxo original descrito na seção preservada abaixo, sem ampliar o escopo funcional deste PR.

Visão

A padronização melhora a revisão, reduz ruído e evita deriva de formatação entre PRs abertos.

Test Plan

  • Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.
Original body

Summary

Add 8 vitest cases for the previously untested launchHermesCommand helper in ui-tui/src/lib/externalCli.ts.

Coverage

  • spawns default hermes binary with stdio: 'inherit'
  • honors HERMES_BIN env override
  • trims whitespace from HERMES_BIN
  • falls back to default when HERMES_BIN is empty/whitespace-only
  • resolves with { code: 0 } on clean exit
  • resolves with non-zero exit code
  • resolves with { code: null, error } on spawn error event
  • passes args through unchanged

Test plan

  • npx vitest run src/__tests__/externalCli.test.ts (8/8 passed)
  • npx vitest run (662/662 passed across 63 files)

Generated by Hermes Turbo


Generated by Hermes Turbo

Add 8 vitest cases for launchHermesCommand:
- default 'hermes' binary with stdio inherit
- HERMES_BIN env override
- whitespace trimming
- empty/whitespace fallback to default
- exit code 0 / non-zero
- spawn error -> {code: null, error}
- args passthrough
Copilot AI review requested due to automatic review settings May 9, 2026 05:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds Vitest coverage for launchHermesCommand in ui-tui/src/lib/externalCli.ts, exercising how the helper selects the Hermes binary and how it resolves its result based on child-process events.

Changes:

  • Added a new test suite validating default spawn behavior (hermes, stdio: 'inherit') and argument passthrough.
  • Added tests covering HERMES_BIN override behavior, including trimming and whitespace-only fallback.
  • Added tests for resolution shape on exit (0 and non-zero) and on error events.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels May 9, 2026
@wesleysimplicio

Copy link
Copy Markdown
Contributor Author

Closing non-fix PR as requested — mantendo apenas PRs de fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants