test: Add regression coverage for RedisVL CLI foundation contracts#596
test: Add regression coverage for RedisVL CLI foundation contracts#596
Conversation
- add_json_output_flag for --json on argparse parsers - cli_print_json for single-object stdout; bytes-safe default Made-with: Cursor
Print {"version": <package>} to stdout; --json overrides --short
Made-with: Cursor
- tests for add_json_output_flag, cli_print_json, Version Made-with: Cursor
Align rvl index and stats: usage problems exit 2 with messages on stderr, Redis/search failures exit 1, success on stdout. Share schema and RedisSearchError handling via redisvl.cli.utils helpers.
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
…extending edit to cli mcp test too
Move SCHEMA_INPUT_ERRORS handling into _connect_to_index (match index CLI) so ValueError from the stats display path is not misclassified as exit 2. Made-with: Cursor
Co-authored-by: Vishal Bala <vishal-bala@users.noreply.github.com>
Co-authored-by: Vishal Bala <vishal-bala@users.noreply.github.com>
Move SCHEMA_INPUT_ERRORS, exit_schema_input_error, and exit_redis_search_error from utils into index.py and stats.py so CLI behavior is visible next to each command. utils keeps URL and argparse helpers only. Made-with: Cursor
This reverts commit ddcd751.
Made-with: Cursor
…i-regression-test
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
…odes rather than soft asserts
vishal-bala
left a comment
vishal-bala
left a comment
There was a problem hiding this comment.
There's a decent amount of structure in place to fake the Redis part of the tests, but when we run the tests, we typically have a Redis instance available. Would it simplify a lot of the code to just use a real Redis instance instead of faking it, or do you think that's messier than it's worth?
| assert re.search(rf"^\s*{re.escape(name)}\s+", help_text, re.MULTILINE) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("argv", [["rvl", "index", "--help"], ["rvl", "index", "-h"]]) |
There was a problem hiding this comment.
It looks like all of these tests have basically the same shape, just different parameters - can we just have the function written once with all of the parameters in one list? That list can have comments embedded to show that there are different groups of arguments, but the main goal of DRY code would be achieved.
| def _raise(exc: BaseException): | ||
| """Return a zero-arg callable that raises ``exc``.""" | ||
|
|
||
| def _do(): | ||
| raise exc | ||
|
|
||
| return _do |
There was a problem hiding this comment.
Does this get used anywhere? Wasn't clear from scanning the code.
3 participants
Changes
In this PR, we reinforce our test coverage of redisVL CLI foundation contracts.
For CLI foundation contracts, the following were verified for correctness:
Note: This PR merges commits from:
Note
Low Risk
Low risk because changes are test-only, but they assert stricter CLI stdout/stderr and exit-code contracts that could fail in CI if CLI behavior differs across environments.
Overview
Strengthens unit/regression coverage for the
rvlCLI, focusing on foundation contracts: help discoverability, exit codes, and strict stdout vs stderr behavior.Adds extensive tests for
rvl,rvl index, andrvl statscovering usage errors (exit2), runtime/Redis failures (exit1), and success paths, including stable JSON output shapes (single-line JSON, ordered keys, no banner text) versus human-readable table output. Also introduces subprocess smoke tests viapython -m redisvl.cli.runnerto verify the installed entrypoint wiring and help output end-to-end.Reviewed by Cursor Bugbot for commit bd2de14. Bugbot is set up for automated code reviews on this repo. Configure here.