Skip to content

fix(tests,admin-cli): output of rack/switch show tests - #5580

Merged
poroh merged 1 commit into
NVIDIA:mainfrom
poroh:fix-admin-cli-test-output
Sep 1, 2026
Merged

fix(tests,admin-cli): output of rack/switch show tests#5580
poroh merged 1 commit into
NVIDIA:mainfrom
poroh:fix-admin-cli-test-output

Conversation

@poroh

@poroh poroh commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Suppress print to stdout during test run in rack/switch admin cli tests, added assertions for these tests.

Related issues

N/A

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Additional Notes

Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
@poroh
poroh requested a review from a team as a code owner August 31, 2026 17:00
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 12708969-dd8e-434b-8d3e-7706dd24f993

📥 Commits

Reviewing files that changed from the base of the PR and between 58f5905 and 7a8da7d.

📒 Files selected for processing (2)
  • crates/admin-cli/src/rack/show/cmd.rs
  • crates/admin-cli/src/switch/show/cmd.rs

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.


Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability of rack details output across JSON, YAML, table, and CSV formats.
    • Rendering errors are now reported correctly instead of being silently ignored.
    • Ensured formatted output can be captured and validated consistently.
    • Improved switch details output validation for key information.

Walkthrough

The rack show command now writes all output formats through supplied writers and propagates rendering errors. Rack and switch formatting tests capture output and validate its contents.

Changes

CLI output rendering

Layer / File(s) Summary
Rack writer-based rendering
crates/admin-cli/src/rack/show/cmd.rs
Rack show functions now accept writers. JSON, YAML, table, and CSV output use the supplied writer and return rendering errors.
Captured output validation
crates/admin-cli/src/rack/show/cmd.rs, crates/admin-cli/src/switch/show/cmd.rs
Tests capture rendered output and assert serialized data, table values, CSV headers, component identifiers, and switch detail fields.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7a8da

This change only adjusts rack and switch CLI tests to suppress unintended output and add assertions; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description accurately states that the rack and switch admin CLI tests suppress stdout output and add assertions.
Title check ✅ Passed The title clearly identifies the fix to rack and switch show test output and matches the main changeset.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7a8da7d4ee

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

];
show_table(&outputs);
let mut rendered = Vec::new();
show_table(&outputs, &mut rendered).unwrap();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Exercise the public rack command in the table test

Calling show_table directly and checking only prefixes through the Compute Trays column allows regressions in command dispatch, headers, and the Power Shelves/Switches cells to pass. Exercise the public rack show command and assert the rendered headers as well as populated and empty values for every column, as required for user-visible CLI table changes.

AGENTS.md reference: AGENTS.md:L147-L149

Useful? React with 👍 / 👎.

@poroh
poroh merged commit 1cf9a4f into NVIDIA:main Sep 1, 2026
65 checks passed
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.

2 participants