Skip to content

test(cli-framework): specify terminal capabilities in hyperlink tests - #9664

Open
PresentJay wants to merge 1 commit into
ParadevAI:mainfrom
PresentJay:test/cli-terminal-links
Open

PresentJay wants to merge 1 commit into
ParadevAI:mainfrom
PresentJay:test/cli-terminal-links

Conversation

@PresentJay

@PresentJay PresentJay commented Nov 12, 2026 •

Copy link
Copy Markdown

What & why

The hyperlink tests set stdout to a TTY but inherit TERM. Under TERM=dumb the formatter correctly emits plain text and two assertions fail. Set the terminal type explicitly for the table tests, restore the caller’s environment afterward, and exercise plain URLs on a dumb TTY.

How I tested it

  • On this branch, macOS arm64 with Bun 1.4.3 and TERM=dumb: from packages/cli-framework, bun test src/output.test.ts — 15 pass, 0 fail.
  • This patch was also part of the combined macOS verification for fix(desktop): let IME users confirm text without submitting a chat answer #9625: all 30 repository test tasks passed in one uncached run, with default workers and timeouts. Repo-wide lint and typecheck passed on that combined checkout.

Checklist

  • Conventional PR title
  • Lint and typecheck passed on the combined verification checkout
  • Allow maintainer edits
  • Signed CLA

Summary by cubic

Makes hyperlink table tests deterministic by setting the terminal type explicitly instead of inheriting TERM from the environment.

  • Sets TERM=xterm-256color for table tests and restores the caller's environment afterward.
  • Adds a test asserting plain URLs are emitted when TERM=dumb.

Written for commit 8575bf5. Summary will update on new commits.

View guided diff

Summary by CodeRabbit

  • Tests
    • Added coverage to verify URLs are printed without hyperlink formatting in dumb terminals.

@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The output tests now control TERM and stdout.isTTY state. They set a standard terminal value for table tests and verify URL output without an OSC 8 hyperlink sequence when TERM is dumb.

Changes: Terminal output test coverage

Layer / File(s) Summary
Terminal test setup and URL coverage
packages/cli-framework/src/output.test.ts
Tests restore TERM and stdout.isTTY after each test. Table tests set TERM to xterm-256color. A new test checks that a TTY with TERM set to dumb prints a URL without an OSC 8 hyperlink sequence.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: harshithmullapudi

Merge Risk: Minimal · up to 8575b

This change only makes the CLI framework's output tests deterministic across terminal environments. It does not alter runtime behavior, and no merge-blocking risk remains.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the change: it adds explicit terminal capability settings to the cli-framework hyperlink tests. The raw summary confirms the output tests now set TERM to xterm-256color and restore it,…
Description check Passed The description follows the template sections: What & why, How I tested it, and Checklist. The What & why section states the problem (TTY stdout with inherited TERM under TERM=dumb) and the fix. It do…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the change: it adds explicit terminal capability settings to the cli-framework hyperlink tests. The raw summary confirms the output tests now set TERM to xterm-256color and restore it, and add a dumb-terminal URL test. The title matches the main change and uses the conventional commit format from the repository template. It omits the dumb-terminal test, which is a minor detail.

Full details: Description check

Explanation

The description follows the template sections: What & why, How I tested it, and Checklist. The What & why section states the problem (TTY stdout with inherited TERM under TERM=dumb) and the fix. It does not link an issue, which the template asks for when one exists. The testing section gives a concrete command and results. The checklist is filled in. Maintainer edits and CLA are checked. The description does not include screenshots, but those apply to user-visible changes only, and this is a test-only change. The description does not explicitly state that lint and typecheck were run on this branch alone; it reports them only for the combined checkout.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 705f5afc-64fc-4ee0-8261-b814ad5897ae
Commits

Reviewing files that changed from the base of the PR and between e9dc091 and 8575bf5.

Files selected for processing: 1

  • packages/cli-framework/src/output.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

This branch has not been deployed

No deployments
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.

1 participant