use console.print_json#571
Conversation
WalkthroughThe changes add minor formatting improvements to an exception handling decorator, enhance JSON output printing by using a dedicated JSON print method, introduce a new JSON comparison utility function, and update multiple test assertions to use this utility for robust JSON equality checks. No logic or control flow is changed. Changes
Estimated code review effort2 (~15 minutes) Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
bennyz
force-pushed
the
add-raw-json
branch
2 times, most recently
from
dcb9efe to
e04f6bb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py (1)
28-30: Good addition of JSON comparison utility.This helper function improves test robustness by comparing parsed JSON objects rather than raw strings, making tests less sensitive to formatting differences.
Consider moving this utility function to a shared test module since it's duplicated in both
get_test.pyandcreate_test.py:# In a new file like test_utils.py import json def json_equal(a: str, b: str) -> bool: return json.loads(a) == json.loads(b)Then import it in both test files to avoid duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py(4 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py(11 hunks)packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py(2 hunks)packages/jumpstarter-cli-common/jumpstarter_cli_common/print.py(1 hunks)packages/jumpstarter-cli/jumpstarter_cli/login.py(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/jumpstarter-cli-common/jumpstarter_cli_common/print.py
- packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py
- packages/jumpstarter-cli/jumpstarter_cli/login.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py (1)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py (1)
json_equal(375-376)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (9)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py (9)
1-1: LGTM! Import addition is correct.The
jsonimport is necessary for the newjson_equalhelper function and is properly placed.
103-103: LGTM! Improved JSON assertion.Using
json_equalmakes the test more robust against formatting differences while ensuring semantic correctness.
255-255: LGTM! Consistent JSON assertion improvement.The change to
json_equalfollows the same improvement pattern for testing JSON list output.
283-283: LGTM! Consistent improvement for empty list testing.The
json_equalusage is properly applied to empty list scenarios as well.
360-360: LGTM! Consistent JSON testing improvement for exporters.The pattern is consistently applied across all resource types.
475-475: LGTM! Consistent application to exporter devices testing.The improvement pattern is properly extended to exporter devices functionality.
620-620: LGTM! Consistent improvement for exporters list.The pattern continues to be applied consistently across all list operations.
788-788: LGTM! Consistent improvement for exporters devices list.The pattern is properly applied to complex list scenarios with nested device data.
979-979: LGTM! Complete coverage of lease testing improvements.The
json_equalpattern has been consistently applied to both individual lease and lease list testing, completing the comprehensive improvement across all resource types.Also applies to: 1181-1181
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/test_utils.py (2)
8-9: Consider using json.JSONDecodeError for more specific exception handling.Using the more specific
json.JSONDecodeErrorinstead of the broadValueErrorwould be more precise, asjson.loads()raisesJSONDecodeError(a subclass ofValueError) for invalid JSON.- except ValueError: + except json.JSONDecodeError:
13-14: Consider using json.JSONDecodeError for consistency.For consistency with the outer exception handling, consider using
json.JSONDecodeErrorhere as well.- except ValueError: + except json.JSONDecodeError:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py(3 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py(10 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/test_utils.py(1 hunks)packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py(2 hunks)packages/jumpstarter-cli-common/jumpstarter_cli_common/print.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/jumpstarter-cli-common/jumpstarter_cli_common/exceptions.py
- packages/jumpstarter-cli-common/jumpstarter_cli_common/print.py
🧰 Additional context used
🧠 Learnings (1)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py (1)
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: e2e
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (4)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/test_utils.py (1)
4-17: Well-designed JSON comparison utility with robust error handling.The
json_equalfunction elegantly handles edge cases like double-encoded JSON and malformed input by using a sentinel object for failed parsing attempts. This provides more reliable JSON comparison than direct string equality.packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py (3)
16-16: Good addition of the json_equal import.The import is correctly placed and enables more robust JSON comparison in the test assertions.
188-188: Improved JSON comparison robustness.Replacing direct string equality with
json_equalmakes the test more resilient to formatting differences while maintaining the same semantic validation.
353-353: Consistent application of robust JSON comparison.Good consistency in applying the
json_equalutility for exporter JSON validation as well.
2 participants
Summary by CodeRabbit