test(cli): add direct unit tests for CLI layout renderers#1015
test(cli): add direct unit tests for CLI layout renderers#10157vignesh wants to merge 2 commits intoTracer-Cloud:mainfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds direct unit tests for the CLI’s custom Rich-based help/landing renderers and introduces a command-metadata sync guard to catch root command drift (Fixes #875).
Changes:
- Add
tests/cli/test_layout.pyto exerciserender_help(),render_landing(), andRichGroup.format_help(). - Add a test ensuring
_HELP_COMMANDSstays aligned with the actual registered root commands. - Update CLI layout metadata to include the
guardrailscommand in help and landing “Quick start” content.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/cli/test_layout.py | Adds focused unit tests for Rich CLI layout rendering and command drift detection. |
| app/cli/layout.py | Updates help/landing command lists to include guardrails. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for label, description in _HELP_COMMANDS: | ||
| assert label in output | ||
| assert description in output | ||
|
|
There was a problem hiding this comment.
output is not normalized here, so Rich line-wrapping can split long descriptions (e.g., the update help text) across newlines, making assert description in output fail depending on console width. Consider normalizing whitespace (like _normalized_output) before asserting, or otherwise making the assertions robust to wrapped output.
| from unittest.mock import MagicMock | ||
|
|
There was a problem hiding this comment.
Unused import: MagicMock is not referenced in this test module. Even though tests ignore F401, removing it will keep the file cleaner and avoid confusion.
| from unittest.mock import MagicMock |
Greptile SummaryThis PR adds direct unit tests for Confidence Score: 5/5Safe to merge — all findings are P2 style/cleanup with no correctness or reliability impact. Both findings are P2: one unused import silently ignored by the per-file ruff config, and one inconsistency in output normalization between two test helpers. Neither affects test correctness or production behavior. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[test_render_help_shows_root_commands] -->|calls| B[render_help]
B -->|reads| C[_HELP_COMMANDS]
B -->|reads| D[_SHORT_OPTIONS]
E[test_render_landing_shows_root_commands_and_header] -->|calls| F[render_landing]
F -->|reads| G[_LANDING_COMMANDS]
F -->|reads| D
H[test_help_command_names_match_layout_metadata] -->|reads| I[cli.commands.keys]
H -->|compares against| C
J[test_rich_group_format_help_delegates_to_render_help] -->|monkeypatches| B
J -->|invokes| K[RichGroup.format_help]
K -->|delegates to| B
Reviews (1): Last reviewed commit: "test(cli): cover layout renderers" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -0,0 +1,75 @@ | |||
| from __future__ import annotations | |||
|
|
|||
| from unittest.mock import MagicMock | |||
There was a problem hiding this comment.
MagicMock
MagicMock is imported but never referenced anywhere in the file. The ruff.toml per-file rule "tests/**/*.py" = ["F401", ...] silently suppresses the warning, but it's still dead code that could mislead future readers into thinking a mock is in use.
| def test_render_help_shows_root_commands(capsys) -> None: | ||
| render_help() | ||
| output = capsys.readouterr().out | ||
|
|
||
| assert "Usage: opensre [OPTIONS] COMMAND [ARGS]..." in output | ||
| assert "Commands:" in output | ||
| assert "Options:" in output | ||
|
|
||
| for label, description in _HELP_COMMANDS: | ||
| assert label in output | ||
| assert description in output | ||
|
|
||
| for label, description in _SHORT_OPTIONS: | ||
| assert label in output | ||
| assert description in output |
There was a problem hiding this comment.
test_render_landing_shows_root_commands_and_header passes the captured output through _normalized_output() (collapsing all whitespace) before asserting, but test_render_help_shows_root_commands checks the raw output directly. With Rich's default 80-column non-TTY width, the help output currently fits without wrapping — but any future widening of labels or descriptions could silently break only this test while the landing test stays green. Consider applying _normalized_output consistently to both, or adding a comment noting why the raw form is safe here.
|
@VaibhavUpreti the pr is ready for review! |
Fixes #875
Added direct tests for the CLI layout in
app/cli/layout.py:render_help()render_landing()RichGroup.format_help()Demo/Screenshot for feature changes and bug fixes -
Validation run:
python -m pytest tests/cli/test_layout.py -qpython -m pytest tests/cli -qpython -m ruff check app/ tests/python -m ruff format --check app/ tests/python -m mypy app/Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
I added focused tests for the CLI layout renderers and command sync, using content checks instead of full output snapshots so the tests stay stable.
Checklist before requesting a review