From bd3f5f1b5682fe96a13fc526354017b0b991d78f Mon Sep 17 00:00:00 2001 From: limjoobin Date: Fri, 24 Apr 2026 18:51:57 +0800 Subject: [PATCH 01/36] feat(cli): add JSON output helpers for rvl - add_json_output_flag for --json on argparse parsers - cli_print_json for single-object stdout; bytes-safe default Made-with: Cursor --- redisvl/cli/utils.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/redisvl/cli/utils.py b/redisvl/cli/utils.py index 8de7e1ae..880b0885 100644 --- a/redisvl/cli/utils.py +++ b/redisvl/cli/utils.py @@ -1,6 +1,7 @@ +import json import os from argparse import ArgumentParser, Namespace -from typing import Optional +from typing import Any, Mapping, Optional from urllib.parse import quote, urlparse, urlunparse from redisvl.redis.constants import REDIS_URL_ENV_VAR @@ -92,3 +93,32 @@ def add_index_parsing_options(parser: ArgumentParser) -> ArgumentParser: default=None, ) return parser + + +def _cli_json_default(obj: object) -> object: + """Handle common non-JSON-native types (e.g. bytes from Redis) for cli_print_json.""" + if isinstance(obj, bytes): + return obj.decode("utf-8", errors="replace") + raise TypeError(f"Object of type {type(obj).__name__} is not JSON serializable") + + +def add_json_output_flag(parser: ArgumentParser) -> ArgumentParser: + """Register ``--json`` for machine-readable stdout (one JSON object on success).""" + parser.add_argument( + "--json", + action="store_true", + dest="json", + default=False, + help="Print success output as JSON to stdout", + ) + return parser + + +def cli_print_json(data: Mapping[str, Any]) -> None: + """Write a single JSON object to stdout (deterministic key order for tests and scripts).""" + print( + json.dumps( + data, + default=_cli_json_default + ) + ) From ddcd751668cecd21929d4f95f2ab0e68d92eaee4 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Fri, 24 Apr 2026 18:51:59 +0800 Subject: [PATCH 02/36] feat(cli): add --json to rvl version Print {"version": } to stdout; --json overrides --short Made-with: Cursor --- redisvl/cli/version.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/redisvl/cli/version.py b/redisvl/cli/version.py index 13facbcd..ec6589c3 100644 --- a/redisvl/cli/version.py +++ b/redisvl/cli/version.py @@ -3,6 +3,7 @@ from argparse import Namespace from redisvl import __version__ +from redisvl.cli.utils import add_json_output_flag, cli_print_json from redisvl.utils.log import get_logger logger = get_logger("[RedisVL]") @@ -21,11 +22,15 @@ def __init__(self): parser.add_argument( "-s", "--short", help="Print only the version number", action="store_true" ) + parser = add_json_output_flag(parser) args = parser.parse_args(sys.argv[2:]) self.version(args) def version(self, args: Namespace): + if args.json: + cli_print_json({"version": __version__}) + return if args.short: print(__version__) else: From 96209c94eb7ba0a4ce7d53ab29f643ad8b5a2472 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Fri, 24 Apr 2026 18:52:02 +0800 Subject: [PATCH 03/36] test(cli): cover rvl version --json and JSON CLI helpers - tests for add_json_output_flag, cli_print_json, Version Made-with: Cursor --- tests/unit/test_cli_utils.py | 47 ++++++++++++++++++++++++++- tests/unit/test_cli_version.py | 59 ++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_cli_version.py diff --git a/tests/unit/test_cli_utils.py b/tests/unit/test_cli_utils.py index e3a8d407..0007730c 100644 --- a/tests/unit/test_cli_utils.py +++ b/tests/unit/test_cli_utils.py @@ -1,9 +1,10 @@ +import json from argparse import ArgumentParser from typing import Optional import pytest -from redisvl.cli.utils import add_index_parsing_options, create_redis_url +from redisvl.cli.utils import add_index_parsing_options, add_json_output_flag, cli_print_json, create_redis_url @pytest.fixture @@ -102,3 +103,47 @@ def test_create_redis_url_resolves_connection_sources( monkeypatch.setenv("REDIS_URL", env_url) assert create_redis_url(parse_args(argv)) == expected + + +def test_add_json_output_flag_absent_is_false(): + """``add_json_output_flag`` leaves ``args.json`` false when ``--json`` is omitted. + + Expected: callers can treat the default as non-JSON (human-oriented) output. + """ + parser = add_json_output_flag(ArgumentParser()) + args = parser.parse_args([]) + assert args.json is False + + +def test_add_json_output_flag_present_is_true(): + """``add_json_output_flag`` sets ``args.json`` true when ``--json`` is passed. + + Expected: parsed args reflect machine-readable JSON mode. + """ + parser = add_json_output_flag(ArgumentParser()) + args = parser.parse_args(["--json"]) + assert args.json is True + + +def test_cli_print_json_writes_single_json_object(capsys): + """``cli_print_json`` writes exactly one JSON object to stdout for a string-only dict. + + Expected: stdout is a single line parseable by ``json.loads``, round-tripping the + payload, with no extra newlines beyond what ``print`` adds for one line. + """ + payload = {"version": "0.0.0"} + cli_print_json(payload) + out = capsys.readouterr().out.strip() + assert json.loads(out) == payload + assert out.count("\n") == 0 + + +def test_cli_print_json_encodes_bytes_values(capsys): + """``cli_print_json`` serializes ``bytes`` values via the JSON default handler. + + Invalid UTF-8 byte sequences are replaced (U+FFFD) in the decoded string, matching + ``_cli_json_default`` so future Redis-centric payloads can be emitted safely. + """ + cli_print_json({"blob": b"abc\xff"}) + out = capsys.readouterr().out.strip() + assert json.loads(out) == {"blob": "abc\ufffd"} diff --git a/tests/unit/test_cli_version.py b/tests/unit/test_cli_version.py new file mode 100644 index 00000000..18b05d66 --- /dev/null +++ b/tests/unit/test_cli_version.py @@ -0,0 +1,59 @@ +import json +import sys + +import pytest + +from redisvl import __version__ +from redisvl.cli.version import Version + + +def test_version_json_prints_package_version(monkeypatch, capsys): + """``rvl version --json`` prints only a JSON object with the package version. + + Expected: stdout is a single JSON object with top-level key ``version`` and a + string value equal to ``redisvl.__version__``; no other keys. + """ + monkeypatch.setattr(sys, "argv", ["rvl", "version", "--json"]) + Version() + out = capsys.readouterr().out.strip() + data = json.loads(out) + assert set(data.keys()) == {"version"} + assert data["version"] == __version__ + + +def test_version_short_prints_plain_version(monkeypatch, capsys): + """``rvl version --short`` prints the bare version string (no JSON, no log prefix on stdout). + + Expected: stdout strip equals ``__version__`` only. + """ + monkeypatch.setattr(sys, "argv", ["rvl", "version", "--short"]) + Version() + assert capsys.readouterr().out.strip() == __version__ + + +@pytest.mark.parametrize( + "extra", + [ + pytest.param(["--json", "--short"], id="json-then-short"), + pytest.param(["--short", "--json"], id="short-then-json"), + ], +) +def test_version_json_overrides_short(monkeypatch, capsys, extra): + """If both ``--json`` and ``--short`` are set, JSON output wins regardless of order. + + Expected: same as ``--json`` alone: one JSON object ``{"version": }``. + """ + monkeypatch.setattr(sys, "argv", ["rvl", "version", *extra]) + Version() + out = capsys.readouterr().out.strip() + data = json.loads(out) + assert data == {"version": __version__} + + +def test_version_default_does_not_raise(monkeypatch): + """``rvl version`` with no extra flags runs without raising (default human/log path). + + Expected: ``Version()`` completes; stdout may be empty while the version is logged. + """ + monkeypatch.setattr(sys, "argv", ["rvl", "version"]) + Version() From 89bd9a14f4ed764363c65609b20e6b2cf4fbe6f9 Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Wed, 22 Apr 2026 22:54:15 +0800 Subject: [PATCH 04/36] fix(cli): top-level exit codes and stderr --- redisvl/cli/main.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/redisvl/cli/main.py b/redisvl/cli/main.py index dbed65f3..90458d27 100644 --- a/redisvl/cli/main.py +++ b/redisvl/cli/main.py @@ -30,29 +30,35 @@ def __init__(self): parser.add_argument("command", help="Subcommand to run") if len(sys.argv) < 2: - parser.print_help() - exit(0) + parser.print_help(sys.stdout) + sys.exit(0) args = parser.parse_args(sys.argv[1:2]) + if not hasattr(self, args.command): - parser.print_help() - exit(0) - getattr(self, args.command)() + print(f"Unknown command: {args.command}\n", file=sys.stderr) + parser.print_help(sys.stderr) + sys.exit(2) + + try: + getattr(self, args.command)() + except Exception as e: + print(e, file=sys.stderr) + sys.exit(1) def index(self): Index() - exit(0) + sys.exit(0) def mcp(self): from redisvl.cli.mcp import MCP - MCP() - exit(0) + sys.exit(0) def version(self): Version() - exit(0) + sys.exit(0) def stats(self): Stats() - exit(0) + sys.exit(0) From 1b0789dd645eefbc178858e7a2a3065205d5650b Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Thu, 23 Apr 2026 16:39:01 +0800 Subject: [PATCH 05/36] fix(cli): standardize exit codes and stderr for index and stats commands 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. --- redisvl/cli/index.py | 62 ++++++++++++++++++++++++++++++++------------ redisvl/cli/stats.py | 29 ++++++++++++++++----- redisvl/cli/utils.py | 39 ++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 23 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index dea8787d..c5715959 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -2,7 +2,14 @@ import sys from argparse import Namespace -from redisvl.cli.utils import add_index_parsing_options, create_redis_url +from redisvl.cli.utils import ( + SCHEMA_INPUT_ERRORS, + add_index_parsing_options, + create_redis_url, + exit_redis_search_error, + exit_schema_input_error, +) +from redisvl.exceptions import RedisSearchError from redisvl.index import SearchIndex from redisvl.redis.connection import RedisConnectionFactory from redisvl.redis.utils import convert_bytes, make_dict @@ -33,26 +40,37 @@ def __init__(self): parser = add_index_parsing_options(parser) args = parser.parse_args(sys.argv[2:]) + if not hasattr(self, args.command): - parser.print_help() - exit(0) + print(f"Unknown command: {args.command}\n", file=sys.stderr) + parser.print_help(sys.stderr) + sys.exit(2) + try: getattr(self, args.command)(args) except Exception as e: - logger.error(e) - exit(0) + logger.error(e, exc_info=True) + print(str(e), file=sys.stderr) + sys.exit(1) def create(self, args: Namespace): """Create an index. Usage: - rvl index create -i | -s + rvl index create -s """ if not args.schema: - print("Schema must be provided to create an index") - exit(1) - index = SearchIndex.from_yaml(args.schema, redis_url=create_redis_url(args)) - index.create() + print("Schema must be provided to create an index", file=sys.stderr) + sys.exit(2) + + index: SearchIndex | None = None + try: + index = SearchIndex.from_yaml(args.schema, redis_url=create_redis_url(args)) + index.create() + except SCHEMA_INPUT_ERRORS as e: + exit_schema_input_error(args, e) + except RedisSearchError as e: + exit_redis_search_error(args, index, e) print("Index created successfully") def info(self, args: Namespace): @@ -61,8 +79,14 @@ def info(self, args: Namespace): Usage: rvl index info -i | -s """ - index = self._connect_to_index(args) - _display_in_table(index.info()) + index: SearchIndex | None = None + try: + index = self._connect_to_index(args) + _display_in_table(index.info()) + except SCHEMA_INPUT_ERRORS as e: + exit_schema_input_error(args, e) + except RedisSearchError as e: + exit_redis_search_error(args, index, e) def listall(self, args: Namespace): """List all indices. @@ -83,8 +107,14 @@ def delete(self, args: Namespace, drop=False): Usage: rvl index delete -i | -s """ - index = self._connect_to_index(args) - index.delete(drop=drop) + index: SearchIndex | None = None + try: + index = self._connect_to_index(args) + index.delete(drop=drop) + except SCHEMA_INPUT_ERRORS as e: + exit_schema_input_error(args, e) + except RedisSearchError as e: + exit_redis_search_error(args, index, e) print("Index deleted successfully") def destroy(self, args: Namespace): @@ -105,8 +135,8 @@ def _connect_to_index(self, args: Namespace) -> SearchIndex: elif args.schema: index = SearchIndex.from_yaml(args.schema, redis_url=redis_url) else: - print("Index name or schema must be provided") - exit(1) + print("Index name or schema must be provided", file=sys.stderr) + sys.exit(2) return index diff --git a/redisvl/cli/stats.py b/redisvl/cli/stats.py index d3bdf9cc..4c1f0a97 100644 --- a/redisvl/cli/stats.py +++ b/redisvl/cli/stats.py @@ -2,7 +2,14 @@ import sys from argparse import Namespace -from redisvl.cli.utils import add_index_parsing_options, create_redis_url +from redisvl.cli.utils import ( + SCHEMA_INPUT_ERRORS, + add_index_parsing_options, + create_redis_url, + exit_redis_search_error, + exit_schema_input_error, +) +from redisvl.exceptions import RedisSearchError from redisvl.index import SearchIndex from redisvl.schema.schema import IndexSchema from redisvl.utils.log import get_logger @@ -43,11 +50,13 @@ def __init__(self): parser = argparse.ArgumentParser(usage=self.usage) parser = add_index_parsing_options(parser) args = parser.parse_args(sys.argv[2:]) + try: self.stats(args) except Exception as e: - logger.error(e) - exit(0) + logger.error(e, exc_info=True) + print(str(e), file=sys.stderr) + sys.exit(1) def stats(self, args: Namespace): """Obtain stats about an index. @@ -55,8 +64,14 @@ def stats(self, args: Namespace): Usage: rvl stats -i | -s """ - index = self._connect_to_index(args) - _display_stats(index.info()) + index: SearchIndex | None = None + try: + index = self._connect_to_index(args) + _display_stats(index.info()) + except SCHEMA_INPUT_ERRORS as e: + exit_schema_input_error(args, e) + except RedisSearchError as e: + exit_redis_search_error(args, index, e) def _connect_to_index(self, args: Namespace) -> SearchIndex: # connect to redis @@ -68,8 +83,8 @@ def _connect_to_index(self, args: Namespace) -> SearchIndex: elif args.schema: index = SearchIndex.from_yaml(args.schema, redis_url=redis_url) else: - logger.error("Index name or schema must be provided") - exit(0) + print("Index name or schema must be provided", file=sys.stderr) + sys.exit(2) return index diff --git a/redisvl/cli/utils.py b/redisvl/cli/utils.py index 6f0da728..566124ea 100644 --- a/redisvl/cli/utils.py +++ b/redisvl/cli/utils.py @@ -1,7 +1,13 @@ import os +import sys from argparse import ArgumentParser, Namespace from urllib.parse import quote, urlparse, urlunparse +import yaml +from pydantic import ValidationError + +from redisvl.exceptions import RedisSearchError +from redisvl.index import SearchIndex from redisvl.redis.constants import REDIS_URL_ENV_VAR from redisvl.utils.log import get_logger @@ -91,3 +97,36 @@ def add_index_parsing_options(parser: ArgumentParser) -> ArgumentParser: default=None, ) return parser + + +# Exceptions commonly raised when loading or validating a schema path (-s). +SCHEMA_INPUT_ERRORS = ( + FileNotFoundError, + ValueError, + yaml.YAMLError, + ValidationError, +) + + +def cli_index_target_name(args: Namespace, index: SearchIndex | None) -> str: + if index is not None: + return index.schema.index.name + return args.index or args.schema or "unknown" + + +def exit_schema_input_error(args: Namespace, exc: BaseException) -> None: + if not args.schema: + raise exc + print(f"Invalid schema file or path ({args.schema!r}): {exc}", file=sys.stderr) + sys.exit(2) + + +def exit_redis_search_error( + args: Namespace, index: SearchIndex | None, exc: RedisSearchError +) -> None: + name = cli_index_target_name(args, index) + print( + f"Redis search operation failed for index {name!r}: {exc}", + file=sys.stderr, + ) + sys.exit(1) From 00637b45ffb438b4f4f260fc53186eec7c2b454a Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:42:00 +0800 Subject: [PATCH 06/36] fix(cli): small fix to print version --- redisvl/cli/version.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/redisvl/cli/version.py b/redisvl/cli/version.py index 13facbcd..b8911b29 100644 --- a/redisvl/cli/version.py +++ b/redisvl/cli/version.py @@ -3,9 +3,6 @@ from argparse import Namespace from redisvl import __version__ -from redisvl.utils.log import get_logger - -logger = get_logger("[RedisVL]") class Version: @@ -29,4 +26,4 @@ def version(self, args: Namespace): if args.short: print(__version__) else: - logger.info(f"RedisVL version {__version__}") + print(f"RedisVL version {__version__}") From 38cb849a4b585333bb581ad9c32052ff8e85bd1a Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Sat, 25 Apr 2026 00:12:10 +0800 Subject: [PATCH 07/36] fix(cli): small edits to error messages Made-with: Cursor --- redisvl/cli/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/redisvl/cli/utils.py b/redisvl/cli/utils.py index 566124ea..77edba6b 100644 --- a/redisvl/cli/utils.py +++ b/redisvl/cli/utils.py @@ -117,7 +117,7 @@ def cli_index_target_name(args: Namespace, index: SearchIndex | None) -> str: def exit_schema_input_error(args: Namespace, exc: BaseException) -> None: if not args.schema: raise exc - print(f"Invalid schema file or path ({args.schema!r}): {exc}", file=sys.stderr) + print(str(exc), file=sys.stderr) sys.exit(2) @@ -126,7 +126,7 @@ def exit_redis_search_error( ) -> None: name = cli_index_target_name(args, index) print( - f"Redis search operation failed for index {name!r}: {exc}", + f"Redis search operation failed for index {name!r}. {exc}", file=sys.stderr, ) sys.exit(1) From 9f3782d5e53a8da06d27c46e343a185214519b1c Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Sat, 25 Apr 2026 00:26:50 +0800 Subject: [PATCH 08/36] chore: apply black formatting to CLI --- redisvl/cli/index.py | 2 +- redisvl/cli/main.py | 5 +++-- redisvl/cli/stats.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index c5715959..2d1bbcd4 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -45,7 +45,7 @@ def __init__(self): print(f"Unknown command: {args.command}\n", file=sys.stderr) parser.print_help(sys.stderr) sys.exit(2) - + try: getattr(self, args.command)(args) except Exception as e: diff --git a/redisvl/cli/main.py b/redisvl/cli/main.py index 90458d27..b986bd57 100644 --- a/redisvl/cli/main.py +++ b/redisvl/cli/main.py @@ -34,12 +34,12 @@ def __init__(self): sys.exit(0) args = parser.parse_args(sys.argv[1:2]) - + if not hasattr(self, args.command): print(f"Unknown command: {args.command}\n", file=sys.stderr) parser.print_help(sys.stderr) sys.exit(2) - + try: getattr(self, args.command)() except Exception as e: @@ -52,6 +52,7 @@ def index(self): def mcp(self): from redisvl.cli.mcp import MCP + MCP() sys.exit(0) diff --git a/redisvl/cli/stats.py b/redisvl/cli/stats.py index 4c1f0a97..7546ee20 100644 --- a/redisvl/cli/stats.py +++ b/redisvl/cli/stats.py @@ -50,7 +50,7 @@ def __init__(self): parser = argparse.ArgumentParser(usage=self.usage) parser = add_index_parsing_options(parser) args = parser.parse_args(sys.argv[2:]) - + try: self.stats(args) except Exception as e: From bead749965148f72088b8f5ee8bc30fdf4b1ef79 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Sun, 26 Apr 2026 15:31:18 +0800 Subject: [PATCH 09/36] feat(cli): add --json to rvl stats with shared _stats_rows Made-with: Cursor --- redisvl/cli/stats.py | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/redisvl/cli/stats.py b/redisvl/cli/stats.py index d3bdf9cc..8977aee6 100644 --- a/redisvl/cli/stats.py +++ b/redisvl/cli/stats.py @@ -2,7 +2,12 @@ import sys from argparse import Namespace -from redisvl.cli.utils import add_index_parsing_options, create_redis_url +from redisvl.cli.utils import ( + add_index_parsing_options, + add_json_output_flag, + cli_print_json, + create_redis_url, +) from redisvl.index import SearchIndex from redisvl.schema.schema import IndexSchema from redisvl.utils.log import get_logger @@ -32,6 +37,17 @@ ] +def _stats_rows(index_info: dict) -> list[tuple[str, str]]: + """Normalize ``index.info()`` for both JSON and table output. + + Returns ordered ``(key, value)`` pairs: keys follow ``STATS_KEYS``, values + are strings (``str(index_info.get(key))``; missing keys become ``"None"``). + For JSON, wrap the result in ``dict(...)``; the table uses the same list of + pairs without conversion. + """ + return [(key, str(index_info.get(key))) for key in STATS_KEYS] + + class Stats: usage = "\n".join( [ @@ -42,6 +58,7 @@ class Stats: def __init__(self): parser = argparse.ArgumentParser(usage=self.usage) parser = add_index_parsing_options(parser) + parser = add_json_output_flag(parser) args = parser.parse_args(sys.argv[2:]) try: self.stats(args) @@ -56,7 +73,12 @@ def stats(self, args: Namespace): rvl stats -i | -s """ index = self._connect_to_index(args) - _display_stats(index.info()) + index_info = index.info() + rows = _stats_rows(index_info) + if args.json: + cli_print_json(dict(rows)) + else: + _display_stats(rows) def _connect_to_index(self, args: Namespace) -> SearchIndex: # connect to redis @@ -74,10 +96,7 @@ def _connect_to_index(self, args: Namespace) -> SearchIndex: return index -def _display_stats(index_info): - # Extracting the statistics - stats_data = [(key, str(index_info.get(key))) for key in STATS_KEYS] - +def _display_stats(stats_data: list[tuple[str, str]]) -> None: # Display the statistics in tabular format print("\nStatistics:") max_key_length = max(len(key) for key, _ in stats_data) From d9bc7e9dc8b25d1ae991f3b35dc4f6e12c88cc28 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Sun, 26 Apr 2026 15:31:21 +0800 Subject: [PATCH 10/36] test: add unit tests for rvl stats CLI Made-with: Cursor --- tests/unit/test_cli_stats.py | 110 +++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 tests/unit/test_cli_stats.py diff --git a/tests/unit/test_cli_stats.py b/tests/unit/test_cli_stats.py new file mode 100644 index 00000000..09a10a39 --- /dev/null +++ b/tests/unit/test_cli_stats.py @@ -0,0 +1,110 @@ +import json +import sys + +import pytest + +from redisvl.cli.stats import STATS_KEYS, Stats, _stats_rows + + +def test_stats_rows_includes_all_stable_top_level_keys_in_order(): + """``_stats_rows({})`` returns the full ordered row list for an empty index info. + + Expected behavior: produces a complete, ``STATS_KEYS``-ordered set of rows + regardless of input; stringifies values at this layer; and represents + missing keys as ``"None"`` so the schema stays stable. + """ + data = dict(_stats_rows({})) + assert list(data.keys()) == list(STATS_KEYS) # column order matches STATS_KEYS + assert all(isinstance(v, str) for v in data.values()) # every value is a string + assert all(data[k] == "None" for k in STATS_KEYS) # missing index_info keys -> "None" + + +def test_stats_json_prints_only_json_to_stdout(monkeypatch, capsys): + """``rvl stats -i --json`` writes only a JSON object to stdout. + + Uses a fake ``SearchIndex`` so no Redis is required. + + Expected behavior: ``--json`` skips ``_display_stats`` and emits one + single-line JSON document with the full ``STATS_KEYS`` schema and + stringified values (e.g. ``num_docs=7`` -> ``"7"``). + + Row order is covered by ``test_stats_rows_*``; JSON key order is covered + by ``test_cli_print_json_preserves_key_order``. + """ + + class FakeIndex: + def __init__(self, *a, **k): + pass + + def info(self): + return {"num_docs": 7} + + monkeypatch.setattr("redisvl.cli.stats.SearchIndex", FakeIndex) + monkeypatch.setattr(sys, "argv", ["rvl", "stats", "-i", "test-idx", "--json"]) + Stats() + out = capsys.readouterr().out.strip() + assert "Statistics" not in out # --json must not emit the table UI text + assert out.count("\n") == 0 # exactly one JSON object on stdout, no extra lines + payload = json.loads(out) + assert set(payload) == set(STATS_KEYS) # same stat keys as the shared schema list + assert payload["num_docs"] == "7" # str() of num_docs from index.info() + + +def test_stats_default_prints_table(monkeypatch, capsys): + """``rvl stats -i `` without ``--json`` still renders the ASCII table. + + Expected behavior: ``Stats.stats`` selects the human-readable branch and + delegates to ``_display_stats``; the ``Statistics:`` banner is the signal + that the table path ran. Guards against the ``--json`` plumbing regressing + the default mode. + """ + + class FakeIndex: + def __init__(self, *a, **k): + pass + + def info(self): + return {"num_docs": 1} + + monkeypatch.setattr("redisvl.cli.stats.SearchIndex", FakeIndex) + monkeypatch.setattr(sys, "argv", ["rvl", "stats", "-i", "test-idx"]) + Stats() + out = capsys.readouterr().out + assert "Statistics:" in out # non-JSON path prints the table header line + + +def test_stats_missing_index_and_schema_exits_zero_without_json(monkeypatch, capsys): + """Without -i/-s, ``_connect_to_index`` logs and ``exit(0)`` s; no JSON leaks. + + Expected behavior: invalid input follows the standard ``rvl`` "log + exit 0" + pattern. ``--json`` does not relax that contract — stdout stays empty so + machine consumers never see a half-formed JSON object. + """ + monkeypatch.setattr(sys, "argv", ["rvl", "stats", "--json"]) + with pytest.raises(SystemExit) as excinfo: + Stats() + assert excinfo.value.code == 0 # CLI's documented "log and exit(0)" contract + assert capsys.readouterr().out == "" # no JSON object emitted on error + + +def test_stats_info_failure_exits_zero_without_json(monkeypatch, capsys): + """If ``index.info()`` raises, ``Stats.__init__`` logs and ``exit(0)`` s; no JSON leaks. + + Expected behavior: ``try/except Exception`` converts backend failures into + ``exit(0)`` (no traceback). With ``--json``, stdout stays empty so "exit 0 + + empty stdout" means "no result", never "malformed result". + """ + + class BoomIndex: + def __init__(self, *a, **k): + pass + + def info(self): + raise RuntimeError("boom") + + monkeypatch.setattr("redisvl.cli.stats.SearchIndex", BoomIndex) + monkeypatch.setattr(sys, "argv", ["rvl", "stats", "-i", "test-idx", "--json"]) + with pytest.raises(SystemExit) as excinfo: + Stats() + assert excinfo.value.code == 0 + assert capsys.readouterr().out == "" From f77532fec7ff2410a5f09fb36481ed734407e353 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Sun, 26 Apr 2026 16:03:19 +0800 Subject: [PATCH 11/36] feat(cli): add --json to rvl index listall Made-with: Cursor --- redisvl/cli/index.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index dea8787d..4f022194 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -2,7 +2,12 @@ import sys from argparse import Namespace -from redisvl.cli.utils import add_index_parsing_options, create_redis_url +from redisvl.cli.utils import ( + add_index_parsing_options, + add_json_output_flag, + cli_print_json, + create_redis_url, +) from redisvl.index import SearchIndex from redisvl.redis.connection import RedisConnectionFactory from redisvl.redis.utils import convert_bytes, make_dict @@ -21,7 +26,7 @@ class Index: "\tcreate Create a new index", "\tdelete Delete an existing index", "\tdestroy Delete an existing index and all of its data", - "\tlistall List all indexes", + "\tlistall List all indexes (use --json for machine output)", "\n", ] ) @@ -31,6 +36,7 @@ def __init__(self): parser.add_argument("command", help="Subcommand to run") parser = add_index_parsing_options(parser) + parser = add_json_output_flag(parser) args = parser.parse_args(sys.argv[2:]) if not hasattr(self, args.command): @@ -68,14 +74,17 @@ def listall(self, args: Namespace): """List all indices. Usage: - rvl index listall + rvl index listall [--json] """ redis_url = create_redis_url(args) conn = RedisConnectionFactory.get_redis_connection(redis_url=redis_url) indices = convert_bytes(conn.execute_command("FT._LIST")) - print("Indices:") - for i, index in enumerate(indices): - print(str(i + 1) + ". " + index) + if args.json: + cli_print_json({"indices": indices}) + else: + print("Indices:") + for i, index in enumerate(indices): + print(str(i + 1) + ". " + index) def delete(self, args: Namespace, drop=False): """Delete an index. From 7276fa5dcaf9470bf4636f8781e08573dc131a6e Mon Sep 17 00:00:00 2001 From: limjoobin Date: Sun, 26 Apr 2026 16:03:21 +0800 Subject: [PATCH 12/36] test: add unit tests for rvl index listall Made-with: Cursor --- tests/unit/test_cli_index.py | 113 +++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 tests/unit/test_cli_index.py diff --git a/tests/unit/test_cli_index.py b/tests/unit/test_cli_index.py new file mode 100644 index 00000000..835250c9 --- /dev/null +++ b/tests/unit/test_cli_index.py @@ -0,0 +1,113 @@ +import json +import sys + +import pytest + +from redisvl.cli.index import Index + + +class _FakeConn: + def __init__(self, result, boom=False): + self._result = result + self._boom = boom + + def execute_command(self, cmd): + assert cmd == "FT._LIST" # listall must query Redis with FT._LIST + if self._boom: + raise RuntimeError("redis unavailable") + return self._result + + +def test_index_listall_json_prints_single_object(monkeypatch, capsys): + """Mocked successful ``FT._LIST`` in ``--json`` mode. + + What: ``listall`` uses ``cli_print_json`` and skips the text banner / loop. + + Expected behavior: exactly one line of parseable JSON on stdout with key + ``indices``; values are the ``convert_bytes`` result of the mock list, in + order; no ``Indices:`` or partial human output; no extra newlines. + """ + + def fake_get(*a, **k): + return _FakeConn([b"idx_a", b"idx_b"]) + + monkeypatch.setattr( + "redisvl.cli.index.RedisConnectionFactory.get_redis_connection", fake_get + ) + monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) + Index() + out = capsys.readouterr().out.strip() + assert "Indices:" not in out # --json must not print the human banner + assert out.count("\n") == 0 # single machine-readable line, nothing else on stdout + payload = json.loads(out) + assert payload == {"indices": ["idx_a", "idx_b"]} # same order/encoding as table path would show + + +def test_index_listall_table_prints_banner(monkeypatch, capsys): + """Default ``listall`` (no ``--json``) uses the text formatter. + + What: non-``--json`` still prints ``Indices:`` and a numbered list from the + same mocked ``FT._LIST`` return. + + Expected behavior: stdout splits into a header line plus one ``N. name`` + line per index, in ``FT._LIST`` order, with no extra lines. + """ + + def fake_get(*a, **k): + return _FakeConn([b"one", b"two"]) + + monkeypatch.setattr( + "redisvl.cli.index.RedisConnectionFactory.get_redis_connection", fake_get + ) + monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall"]) + Index() + out = capsys.readouterr().out + lines = [ln.strip() for ln in out.strip().splitlines()] + assert lines == [ + "Indices:", + "1. one", + "2. two", + ] # exact table output: header then rows matching mock order and labels + + +def test_index_listall_json_empty_indices(monkeypatch, capsys): + """Empty result from ``FT._LIST`` in ``--json`` mode. + + What: empty list after ``convert_bytes`` still forms a valid JSON object. + + Expected behavior: one line ``{"indices": []}`` with no error. + """ + + def fake_get(*a, **k): + return _FakeConn([]) + + monkeypatch.setattr( + "redisvl.cli.index.RedisConnectionFactory.get_redis_connection", fake_get + ) + monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) + Index() + out = capsys.readouterr().out.strip() + assert json.loads(out) == {"indices": []} # empty array is a valid success payload + + +def test_index_listall_execute_error_exits_zero_without_json_stdout(monkeypatch, capsys): + """``FT._LIST`` raises: ``Index`` catches, logs, ``exit(0)``; no JSON leak. + + What: backend failure is handled by the ``Index.__init__`` try/except like + other rvl subcommands, even with ``--json`` requested. + + Expected behavior: process exits 0; stdout is empty (no half-written JSON + from ``cli_print_json``). + """ + + def fake_get(*a, **k): + return _FakeConn([], boom=True) + + monkeypatch.setattr( + "redisvl.cli.index.RedisConnectionFactory.get_redis_connection", fake_get + ) + monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) + with pytest.raises(SystemExit) as excinfo: # exit(0) in Index.__init__ is not a plain return + Index() + assert excinfo.value.code == 0 # "log and exit(0)" CLI contract + assert capsys.readouterr().out == "" # failure before cli_print_json — nothing on stdout From 8f6118ac9ec9d7504848b84a030d810b241dce20 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Sun, 26 Apr 2026 23:21:33 +0800 Subject: [PATCH 13/36] feat(cli): add --json to rvl index info --- redisvl/cli/index.py | 47 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index 4f022194..c80b3db4 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -22,7 +22,7 @@ class Index: [ "rvl index []\n", "Commands:", - "\tinfo Obtain information about an index", + "\tinfo Obtain information about an index (use --json for machine output)", "\tcreate Create a new index", "\tdelete Delete an existing index", "\tdestroy Delete an existing index and all of its data", @@ -65,10 +65,14 @@ def info(self, args: Namespace): """Obtain information about an index. Usage: - rvl index info -i | -s + rvl index info -i | -s [--json] """ index = self._connect_to_index(args) - _display_in_table(index.info()) + index_info = index.info() + if args.json: + cli_print_json(_index_info_for_json(index_info)) + else: + _display_in_table(index_info) def listall(self, args: Namespace): """List all indices. @@ -120,6 +124,43 @@ def _connect_to_index(self, args: Namespace) -> SearchIndex: return index +def _index_info_for_json(index_info: dict) -> dict: + """Build the JSON payload from the same fields shown in table mode.""" + definition = convert_bytes(make_dict(index_info.get("index_definition"))) + attributes = index_info.get("attributes", []) + index_fields = [] + + for attrs in attributes: + attr = ( + convert_bytes(make_dict(attrs)) if isinstance(attrs, (list, tuple)) + else convert_bytes(dict(attrs)) + ) + field = { + "name": attr.get("identifier"), + "attribute": attr.get("attribute"), + "type": attr.get("type"), + } + field_options = { + k: v for k, v in attr.items() + if k not in {"identifier", "attribute", "type"} + } + if field_options: + field["field_options"] = field_options + index_fields.append(field) + + payload = { + "index_information": { + "index_name": index_info.get("index_name"), + "storage_type": definition.get("key_type"), + "prefixes": definition.get("prefixes"), + "index_options": index_info.get("index_options"), + "indexing": index_info.get("indexing"), + }, + "index_fields": index_fields, + } + return convert_bytes(payload) + + def _display_in_table(index_info): print("\n") attributes = index_info.get("attributes", []) From 76d45ccd76f1d8165147efa1db060dcb41f7f441 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Sun, 26 Apr 2026 23:21:36 +0800 Subject: [PATCH 14/36] test: add unit tests for rvl index info --json --- tests/unit/test_cli_index.py | 168 +++++++++++++++++++++++++++++------ 1 file changed, 141 insertions(+), 27 deletions(-) diff --git a/tests/unit/test_cli_index.py b/tests/unit/test_cli_index.py index 835250c9..0a3c2f81 100644 --- a/tests/unit/test_cli_index.py +++ b/tests/unit/test_cli_index.py @@ -3,7 +3,7 @@ import pytest -from redisvl.cli.index import Index +from redisvl.cli.index import Index, _index_info_for_json class _FakeConn: @@ -18,14 +18,10 @@ def execute_command(self, cmd): return self._result -def test_index_listall_json_prints_single_object(monkeypatch, capsys): - """Mocked successful ``FT._LIST`` in ``--json`` mode. +def test_listall_json(monkeypatch, capsys): + """Tests that ``listall --json`` prints machine-readable output only. - What: ``listall`` uses ``cli_print_json`` and skips the text banner / loop. - - Expected behavior: exactly one line of parseable JSON on stdout with key - ``indices``; values are the ``convert_bytes`` result of the mock list, in - order; no ``Indices:`` or partial human output; no extra newlines. + Expected behavior: stdout is one JSON line with ``indices`` in order and no table text. """ def fake_get(*a, **k): @@ -43,14 +39,10 @@ def fake_get(*a, **k): assert payload == {"indices": ["idx_a", "idx_b"]} # same order/encoding as table path would show -def test_index_listall_table_prints_banner(monkeypatch, capsys): - """Default ``listall`` (no ``--json``) uses the text formatter. - - What: non-``--json`` still prints ``Indices:`` and a numbered list from the - same mocked ``FT._LIST`` return. +def test_listall_table(monkeypatch, capsys): + """Tests that default ``listall`` keeps the human-readable table output. - Expected behavior: stdout splits into a header line plus one ``N. name`` - line per index, in ``FT._LIST`` order, with no extra lines. + Expected behavior: stdout matches header + numbered rows in FT._LIST order. """ def fake_get(*a, **k): @@ -70,12 +62,10 @@ def fake_get(*a, **k): ] # exact table output: header then rows matching mock order and labels -def test_index_listall_json_empty_indices(monkeypatch, capsys): - """Empty result from ``FT._LIST`` in ``--json`` mode. - - What: empty list after ``convert_bytes`` still forms a valid JSON object. +def test_listall_json_empty(monkeypatch, capsys): + """Tests that ``listall --json`` handles an empty FT._LIST result. - Expected behavior: one line ``{"indices": []}`` with no error. + Expected behavior: stdout is valid JSON with ``{"indices": []}``. """ def fake_get(*a, **k): @@ -90,14 +80,10 @@ def fake_get(*a, **k): assert json.loads(out) == {"indices": []} # empty array is a valid success payload -def test_index_listall_execute_error_exits_zero_without_json_stdout(monkeypatch, capsys): - """``FT._LIST`` raises: ``Index`` catches, logs, ``exit(0)``; no JSON leak. +def test_listall_json_error(monkeypatch, capsys): + """Tests that ``listall --json`` failure exits cleanly without stdout JSON. - What: backend failure is handled by the ``Index.__init__`` try/except like - other rvl subcommands, even with ``--json`` requested. - - Expected behavior: process exits 0; stdout is empty (no half-written JSON - from ``cli_print_json``). + Expected behavior: ``SystemExit`` code is 0 and stdout is empty. """ def fake_get(*a, **k): @@ -111,3 +97,131 @@ def fake_get(*a, **k): Index() assert excinfo.value.code == 0 # "log and exit(0)" CLI contract assert capsys.readouterr().out == "" # failure before cli_print_json — nothing on stdout + + +def test_info_json_normalize(): + """Tests that ``_index_info_for_json`` maps FT.INFO lists to structured JSON. + + Expected behavior: input is unchanged and output has ``index_information`` + ``index_fields``. + """ + raw = { + "index_name": "test_index", + "index_definition": [ + "key_type", + "HASH", + "prefixes", + ["prefix_a", "prefix_b"], + ], + "attributes": [ + [ + "identifier", + "user", + "attribute", + "user", + "type", + "TAG", + ], + ], + } + before = str(raw) + out = _index_info_for_json(raw) + assert str(raw) == before # not mutated + assert out == { + "index_information": { + "index_name": "test_index", + "storage_type": "HASH", + "prefixes": ["prefix_a", "prefix_b"], + "index_options": None, + "indexing": None, + }, + "index_fields": [ + { + "name": "user", + "attribute": "user", + "type": "TAG", + } + ], + } # exact summary+fields payload, matching what table prints semantically + + +def test_info_json(monkeypatch, capsys): + """Tests that ``info --json`` returns normalized table-equivalent JSON. + + Expected behavior: one parseable JSON line with decoded values and no table banners. + """ + + expected_index_information = { + "index_name": "test-idx", + "storage_type": "HASH", + "prefixes": ["pre"], + "index_options": None, + "indexing": None, + } + expected_field = { + "name": "u", + "attribute": "u", + "type": "TAG", + "field_options": {"NOSTEM": "1"}, + } + + class FakeIndex: + def __init__(self, *a, **k): + pass + + def info(self): + return { + "index_name": b"test-idx", + "index_definition": [ + "key_type", + b"HASH", + "prefixes", + [b"pre"], + ], + "attributes": [ + [ + b"identifier", + b"u", + b"attribute", + b"u", + b"type", + b"TAG", + b"NOSTEM", + b"1", + ], + ], + } + + monkeypatch.setattr("redisvl.cli.index.SearchIndex", FakeIndex) + monkeypatch.setattr( + sys, "argv", ["rvl", "index", "info", "-i", "test-idx", "--json"] + ) + Index() + out = capsys.readouterr().out.strip() + assert out.count("\n") == 0 # single line for machine consumers + payload = json.loads(out) + assert "Index Information:" not in out and "Index Fields:" not in out # --json must not emit table banner text + assert list(payload) == ["index_information", "index_fields"] # top-level sections are stable and ordered + assert payload["index_information"] == expected_index_information # summary section matches table-derived values + assert payload["index_fields"] == [expected_field] # one normalized field row with options + +def test_info_json_error(monkeypatch, capsys): + """Tests that ``info --json`` errors do not emit partial stdout JSON. + + Expected behavior: command exits with code 0 and stdout is empty. + """ + + class BoomIndex: + def __init__(self, *a, **k): + pass + + def info(self): + raise RuntimeError("boom") + + monkeypatch.setattr("redisvl.cli.index.SearchIndex", BoomIndex) + monkeypatch.setattr( + sys, "argv", ["rvl", "index", "info", "-i", "test-idx", "--json"] + ) + with pytest.raises(SystemExit) as excinfo: + Index() + assert excinfo.value.code == 0 # try/except in Index.__init__ + exit(0) + assert capsys.readouterr().out == "" # no partial JSON before the exception From 8b3b2ccee4b910acc782799cec6f76c95564d59b Mon Sep 17 00:00:00 2001 From: limjoobin Date: Mon, 27 Apr 2026 14:54:03 +0800 Subject: [PATCH 15/36] chore: Format index.py and utils.py with black --- redisvl/cli/index.py | 6 ++++-- redisvl/cli/utils.py | 7 +------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index c80b3db4..d1f22019 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -132,7 +132,8 @@ def _index_info_for_json(index_info: dict) -> dict: for attrs in attributes: attr = ( - convert_bytes(make_dict(attrs)) if isinstance(attrs, (list, tuple)) + convert_bytes(make_dict(attrs)) + if isinstance(attrs, (list, tuple)) else convert_bytes(dict(attrs)) ) field = { @@ -141,7 +142,8 @@ def _index_info_for_json(index_info: dict) -> dict: "type": attr.get("type"), } field_options = { - k: v for k, v in attr.items() + k: v + for k, v in attr.items() if k not in {"identifier", "attribute", "type"} } if field_options: diff --git a/redisvl/cli/utils.py b/redisvl/cli/utils.py index 836768aa..8c1169f7 100644 --- a/redisvl/cli/utils.py +++ b/redisvl/cli/utils.py @@ -116,9 +116,4 @@ def add_json_output_flag(parser: ArgumentParser) -> ArgumentParser: def cli_print_json(data: Mapping[str, Any]) -> None: """Write a single JSON object to stdout (deterministic key order for tests and scripts).""" - print( - json.dumps( - data, - default=_cli_json_default - ) - ) + print(json.dumps(data, default=_cli_json_default)) From db1313f72cdd20970f54d973b64fa61ae08938ae Mon Sep 17 00:00:00 2001 From: limjoobin Date: Mon, 27 Apr 2026 15:03:42 +0800 Subject: [PATCH 16/36] fix(types): coerce index info inputs before make_dict in cli index json path --- redisvl/cli/index.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index d1f22019..860b9058 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -126,16 +126,25 @@ def _connect_to_index(self, args: Namespace) -> SearchIndex: def _index_info_for_json(index_info: dict) -> dict: """Build the JSON payload from the same fields shown in table mode.""" - definition = convert_bytes(make_dict(index_info.get("index_definition"))) + definition_src = index_info.get("index_definition") + if isinstance(definition_src, list): + definition = convert_bytes(make_dict(definition_src)) + elif isinstance(definition_src, tuple): + definition = convert_bytes(make_dict(list(definition_src))) + else: + definition = {} attributes = index_info.get("attributes", []) index_fields = [] for attrs in attributes: - attr = ( - convert_bytes(make_dict(attrs)) - if isinstance(attrs, (list, tuple)) - else convert_bytes(dict(attrs)) - ) + if isinstance(attrs, list): + attr = convert_bytes(make_dict(attrs)) + elif isinstance(attrs, tuple): + attr = convert_bytes(make_dict(list(attrs))) + elif isinstance(attrs, dict): + attr = convert_bytes(dict(attrs)) + else: + attr = {} field = { "name": attr.get("identifier"), "attribute": attr.get("attribute"), From ce70d095c60e5c7b5b1d26af7835f600045e8d36 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Mon, 27 Apr 2026 15:42:59 +0800 Subject: [PATCH 17/36] fix(cli): preserve dict index_definition in index info json --- redisvl/cli/index.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index 860b9058..ae0bbc04 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -131,6 +131,8 @@ def _index_info_for_json(index_info: dict) -> dict: definition = convert_bytes(make_dict(definition_src)) elif isinstance(definition_src, tuple): definition = convert_bytes(make_dict(list(definition_src))) + elif isinstance(definition_src, dict): + definition = convert_bytes(dict(definition_src)) else: definition = {} attributes = index_info.get("attributes", []) From 25d0dc72425480282d67b02c9f1ba1e776660a15 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Mon, 27 Apr 2026 16:01:23 +0800 Subject: [PATCH 18/36] fix(cli-stats): preserve numeric/null types in --json output --- redisvl/cli/stats.py | 15 ++++++++------- tests/unit/test_cli_stats.py | 12 ++++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/redisvl/cli/stats.py b/redisvl/cli/stats.py index 8977aee6..95594bf7 100644 --- a/redisvl/cli/stats.py +++ b/redisvl/cli/stats.py @@ -37,15 +37,15 @@ ] -def _stats_rows(index_info: dict) -> list[tuple[str, str]]: +def _stats_rows(index_info: dict) -> list[tuple[str, object]]: """Normalize ``index.info()`` for both JSON and table output. Returns ordered ``(key, value)`` pairs: keys follow ``STATS_KEYS``, values - are strings (``str(index_info.get(key))``; missing keys become ``"None"``). - For JSON, wrap the result in ``dict(...)``; the table uses the same list of - pairs without conversion. + preserve native types from ``index_info``. Missing keys remain ``None``. + For JSON, wrap the result in ``dict(...)``; the table stringifies values + when rendering. """ - return [(key, str(index_info.get(key))) for key in STATS_KEYS] + return [(key, index_info.get(key)) for key in STATS_KEYS] class Stats: @@ -96,7 +96,7 @@ def _connect_to_index(self, args: Namespace) -> SearchIndex: return index -def _display_stats(stats_data: list[tuple[str, str]]) -> None: +def _display_stats(stats_data: list[tuple[str, object]]) -> None: # Display the statistics in tabular format print("\nStatistics:") max_key_length = max(len(key) for key, _ in stats_data) @@ -105,5 +105,6 @@ def _display_stats(stats_data: list[tuple[str, str]]) -> None: print("│ Stat Key │ Value │") # header row print(f"├{horizontal_line}┼────────────┤") # separator row for key, value in stats_data: - print(f"│ {key:<27} │ {value[0:10]:<10} │") # data rows + value_str = str(value) + print(f"│ {key:<27} │ {value_str[0:10]:<10} │") # data rows print(f"╰{horizontal_line}┴────────────╯") # bottom row diff --git a/tests/unit/test_cli_stats.py b/tests/unit/test_cli_stats.py index 09a10a39..c9519757 100644 --- a/tests/unit/test_cli_stats.py +++ b/tests/unit/test_cli_stats.py @@ -10,13 +10,12 @@ def test_stats_rows_includes_all_stable_top_level_keys_in_order(): """``_stats_rows({})`` returns the full ordered row list for an empty index info. Expected behavior: produces a complete, ``STATS_KEYS``-ordered set of rows - regardless of input; stringifies values at this layer; and represents - missing keys as ``"None"`` so the schema stays stable. + regardless of input; preserves value types at this layer; and represents + missing keys as ``None`` so JSON output remains machine-readable. """ data = dict(_stats_rows({})) assert list(data.keys()) == list(STATS_KEYS) # column order matches STATS_KEYS - assert all(isinstance(v, str) for v in data.values()) # every value is a string - assert all(data[k] == "None" for k in STATS_KEYS) # missing index_info keys -> "None" + assert all(data[k] is None for k in STATS_KEYS) # missing index_info keys -> None def test_stats_json_prints_only_json_to_stdout(monkeypatch, capsys): @@ -26,7 +25,7 @@ def test_stats_json_prints_only_json_to_stdout(monkeypatch, capsys): Expected behavior: ``--json`` skips ``_display_stats`` and emits one single-line JSON document with the full ``STATS_KEYS`` schema and - stringified values (e.g. ``num_docs=7`` -> ``"7"``). + native values (e.g. ``num_docs=7`` -> ``7``). Row order is covered by ``test_stats_rows_*``; JSON key order is covered by ``test_cli_print_json_preserves_key_order``. @@ -47,7 +46,8 @@ def info(self): assert out.count("\n") == 0 # exactly one JSON object on stdout, no extra lines payload = json.loads(out) assert set(payload) == set(STATS_KEYS) # same stat keys as the shared schema list - assert payload["num_docs"] == "7" # str() of num_docs from index.info() + assert payload["num_docs"] == 7 # numbers remain numbers for machine consumers + assert payload["num_terms"] is None # missing values become JSON null, not "None" def test_stats_default_prints_table(monkeypatch, capsys): From 5ac9378226d307254514faca5551dd1cf87d1e92 Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Mon, 27 Apr 2026 16:29:55 +0800 Subject: [PATCH 19/36] fix(cli): exit 2 for invalid rvl mcp flags (match argparse default), extending edit to cli mcp test too --- redisvl/cli/mcp.py | 4 ++-- tests/unit/test_cli_mcp.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/redisvl/cli/mcp.py b/redisvl/cli/mcp.py index 376881d6..822efc77 100644 --- a/redisvl/cli/mcp.py +++ b/redisvl/cli/mcp.py @@ -7,11 +7,11 @@ class _MCPArgumentParser(argparse.ArgumentParser): - """ArgumentParser variant that reports usage errors with exit code 1.""" + """ArgumentParser variant that reports usage errors with exit code 2.""" def error(self, message): self.print_usage(sys.stderr) - self.exit(1, "%s: error: %s\n" % (self.prog, message)) + self.exit(2, "%s: error: %s\n" % (self.prog, message)) class MCP: diff --git a/tests/unit/test_cli_mcp.py b/tests/unit/test_cli_mcp.py index e4fddc38..4beb5cf0 100644 --- a/tests/unit/test_cli_mcp.py +++ b/tests/unit/test_cli_mcp.py @@ -464,7 +464,7 @@ def test_mcp_command_rejects_invalid_transport(monkeypatch, capsys): with pytest.raises(SystemExit) as exc_info: module.MCP() - assert exc_info.value.code == 1 + assert exc_info.value.code == 2 out = capsys.readouterr() assert "invalid choice" in out.err or "invalid choice" in out.out From e1d40e3196d07854b90c3013d78c1d6b76d3ebe2 Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Wed, 29 Apr 2026 12:04:39 +0800 Subject: [PATCH 20/36] refactor(cli): narrow index schema errors in _connect_to_index --- redisvl/cli/index.py | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index 2d1bbcd4..9ee83682 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -63,12 +63,13 @@ def create(self, args: Namespace): print("Schema must be provided to create an index", file=sys.stderr) sys.exit(2) - index: SearchIndex | None = None + redis_url = create_redis_url(args) try: - index = SearchIndex.from_yaml(args.schema, redis_url=create_redis_url(args)) - index.create() + index = SearchIndex.from_yaml(args.schema, redis_url=redis_url) except SCHEMA_INPUT_ERRORS as e: exit_schema_input_error(args, e) + try: + index.create() except RedisSearchError as e: exit_redis_search_error(args, index, e) print("Index created successfully") @@ -79,12 +80,9 @@ def info(self, args: Namespace): Usage: rvl index info -i | -s """ - index: SearchIndex | None = None + index = self._connect_to_index(args) try: - index = self._connect_to_index(args) _display_in_table(index.info()) - except SCHEMA_INPUT_ERRORS as e: - exit_schema_input_error(args, e) except RedisSearchError as e: exit_redis_search_error(args, index, e) @@ -107,12 +105,9 @@ def delete(self, args: Namespace, drop=False): Usage: rvl index delete -i | -s """ - index: SearchIndex | None = None + index = self._connect_to_index(args) try: - index = self._connect_to_index(args) index.delete(drop=drop) - except SCHEMA_INPUT_ERRORS as e: - exit_schema_input_error(args, e) except RedisSearchError as e: exit_redis_search_error(args, index, e) print("Index deleted successfully") @@ -126,19 +121,23 @@ def destroy(self, args: Namespace): self.delete(args, drop=True) def _connect_to_index(self, args: Namespace) -> SearchIndex: - # connect to redis redis_url = create_redis_url(args) if args.index: - schema = IndexSchema.from_dict({"index": {"name": args.index}}) - index = SearchIndex(schema=schema, redis_url=redis_url) - elif args.schema: - index = SearchIndex.from_yaml(args.schema, redis_url=redis_url) - else: - print("Index name or schema must be provided", file=sys.stderr) - sys.exit(2) - - return index + try: + schema = IndexSchema.from_dict({"index": {"name": args.index}}) + return SearchIndex(schema=schema, redis_url=redis_url) + except SCHEMA_INPUT_ERRORS as e: + exit_schema_input_error(args, e) + + if args.schema: + try: + return SearchIndex.from_yaml(args.schema, redis_url=redis_url) + except SCHEMA_INPUT_ERRORS as e: + exit_schema_input_error(args, e) + + print("Index name or schema must be provided", file=sys.stderr) + sys.exit(2) def _display_in_table(index_info): From 38fee23d2ed5bd08aee7493358599efa74fad685 Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Wed, 29 Apr 2026 13:03:43 +0800 Subject: [PATCH 21/36] fix(cli): separate stats schema load from index.info() error handling 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 --- redisvl/cli/stats.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/redisvl/cli/stats.py b/redisvl/cli/stats.py index 7546ee20..b96f150d 100644 --- a/redisvl/cli/stats.py +++ b/redisvl/cli/stats.py @@ -64,29 +64,30 @@ def stats(self, args: Namespace): Usage: rvl stats -i | -s """ - index: SearchIndex | None = None + index = self._connect_to_index(args) try: - index = self._connect_to_index(args) _display_stats(index.info()) - except SCHEMA_INPUT_ERRORS as e: - exit_schema_input_error(args, e) except RedisSearchError as e: exit_redis_search_error(args, index, e) def _connect_to_index(self, args: Namespace) -> SearchIndex: - # connect to redis redis_url = create_redis_url(args) if args.index: - schema = IndexSchema.from_dict({"index": {"name": args.index}}) - index = SearchIndex(schema=schema, redis_url=redis_url) - elif args.schema: - index = SearchIndex.from_yaml(args.schema, redis_url=redis_url) - else: - print("Index name or schema must be provided", file=sys.stderr) - sys.exit(2) - - return index + try: + schema = IndexSchema.from_dict({"index": {"name": args.index}}) + return SearchIndex(schema=schema, redis_url=redis_url) + except SCHEMA_INPUT_ERRORS as e: + exit_schema_input_error(args, e) + + if args.schema: + try: + return SearchIndex.from_yaml(args.schema, redis_url=redis_url) + except SCHEMA_INPUT_ERRORS as e: + exit_schema_input_error(args, e) + + print("Index name or schema must be provided", file=sys.stderr) + sys.exit(2) def _display_stats(index_info): From 165b755275cf41577b58700207b44f313b14853d Mon Sep 17 00:00:00 2001 From: limjoobin <38883279+limjoobin@users.noreply.github.com> Date: Wed, 29 Apr 2026 21:08:08 +0800 Subject: [PATCH 22/36] chore: minor refactor for readability Co-authored-by: Vishal Bala --- redisvl/cli/index.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index ae0bbc04..47db9277 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -87,8 +87,8 @@ def listall(self, args: Namespace): cli_print_json({"indices": indices}) else: print("Indices:") - for i, index in enumerate(indices): - print(str(i + 1) + ". " + index) + for i, index in enumerate(indices, start=1): + print(str(i) + ". " + index) def delete(self, args: Namespace, drop=False): """Delete an index. From 316e542e43dd9a177df319bc1f17155c2839fe5f Mon Sep 17 00:00:00 2001 From: limjoobin <38883279+limjoobin@users.noreply.github.com> Date: Wed, 29 Apr 2026 21:09:23 +0800 Subject: [PATCH 23/36] chore: updated parser description for readability Co-authored-by: Vishal Bala --- redisvl/cli/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisvl/cli/utils.py b/redisvl/cli/utils.py index 8c1169f7..73f1e376 100644 --- a/redisvl/cli/utils.py +++ b/redisvl/cli/utils.py @@ -109,7 +109,7 @@ def add_json_output_flag(parser: ArgumentParser) -> ArgumentParser: action="store_true", dest="json", default=False, - help="Print success output as JSON to stdout", + help="Format output (when successful) as machine-readable JSON", ) return parser From 7632de4b91f9bc65e7a2ac2686559b0306ddacd7 Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Wed, 29 Apr 2026 21:35:38 +0800 Subject: [PATCH 24/36] refactor(cli): colocate index/stats schema errors and exit helpers 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 --- redisvl/cli/index.py | 41 ++++++++++++++++++++++++++++++++++------- redisvl/cli/stats.py | 41 ++++++++++++++++++++++++++++++++++------- redisvl/cli/utils.py | 39 --------------------------------------- 3 files changed, 68 insertions(+), 53 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index 9ee83682..15cd28bd 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -2,13 +2,10 @@ import sys from argparse import Namespace -from redisvl.cli.utils import ( - SCHEMA_INPUT_ERRORS, - add_index_parsing_options, - create_redis_url, - exit_redis_search_error, - exit_schema_input_error, -) +import yaml +from pydantic import ValidationError + +from redisvl.cli.utils import add_index_parsing_options, create_redis_url from redisvl.exceptions import RedisSearchError from redisvl.index import SearchIndex from redisvl.redis.connection import RedisConnectionFactory @@ -18,6 +15,36 @@ logger = get_logger("[RedisVL]") +# Exceptions commonly raised when loading or validating a schema path (-s). +SCHEMA_INPUT_ERRORS = ( + FileNotFoundError, + ValueError, + yaml.YAMLError, + ValidationError, +) + + +def exit_schema_input_error(args: Namespace, exc: BaseException) -> None: + if not args.schema: + raise exc + print(str(exc), file=sys.stderr) + sys.exit(2) + + +def exit_redis_search_error( + args: Namespace, index: SearchIndex | None, exc: RedisSearchError +) -> None: + name = ( + index.schema.index.name + if index is not None + else (args.index or args.schema or "unknown") + ) + print( + f"Redis search operation failed for index {name!r}. {exc}", + file=sys.stderr, + ) + sys.exit(1) + class Index: usage = "\n".join( diff --git a/redisvl/cli/stats.py b/redisvl/cli/stats.py index b96f150d..a55beb7d 100644 --- a/redisvl/cli/stats.py +++ b/redisvl/cli/stats.py @@ -2,13 +2,10 @@ import sys from argparse import Namespace -from redisvl.cli.utils import ( - SCHEMA_INPUT_ERRORS, - add_index_parsing_options, - create_redis_url, - exit_redis_search_error, - exit_schema_input_error, -) +import yaml +from pydantic import ValidationError + +from redisvl.cli.utils import add_index_parsing_options, create_redis_url from redisvl.exceptions import RedisSearchError from redisvl.index import SearchIndex from redisvl.schema.schema import IndexSchema @@ -16,6 +13,36 @@ logger = get_logger("[RedisVL]") +# Exceptions commonly raised when loading or validating a schema path (-s). +SCHEMA_INPUT_ERRORS = ( + FileNotFoundError, + ValueError, + yaml.YAMLError, + ValidationError, +) + + +def exit_schema_input_error(args: Namespace, exc: BaseException) -> None: + if not args.schema: + raise exc + print(str(exc), file=sys.stderr) + sys.exit(2) + + +def exit_redis_search_error( + args: Namespace, index: SearchIndex | None, exc: RedisSearchError +) -> None: + name = ( + index.schema.index.name + if index is not None + else (args.index or args.schema or "unknown") + ) + print( + f"Redis search operation failed for index {name!r}. {exc}", + file=sys.stderr, + ) + sys.exit(1) + STATS_KEYS = [ "num_docs", "num_terms", diff --git a/redisvl/cli/utils.py b/redisvl/cli/utils.py index 77edba6b..6f0da728 100644 --- a/redisvl/cli/utils.py +++ b/redisvl/cli/utils.py @@ -1,13 +1,7 @@ import os -import sys from argparse import ArgumentParser, Namespace from urllib.parse import quote, urlparse, urlunparse -import yaml -from pydantic import ValidationError - -from redisvl.exceptions import RedisSearchError -from redisvl.index import SearchIndex from redisvl.redis.constants import REDIS_URL_ENV_VAR from redisvl.utils.log import get_logger @@ -97,36 +91,3 @@ def add_index_parsing_options(parser: ArgumentParser) -> ArgumentParser: default=None, ) return parser - - -# Exceptions commonly raised when loading or validating a schema path (-s). -SCHEMA_INPUT_ERRORS = ( - FileNotFoundError, - ValueError, - yaml.YAMLError, - ValidationError, -) - - -def cli_index_target_name(args: Namespace, index: SearchIndex | None) -> str: - if index is not None: - return index.schema.index.name - return args.index or args.schema or "unknown" - - -def exit_schema_input_error(args: Namespace, exc: BaseException) -> None: - if not args.schema: - raise exc - print(str(exc), file=sys.stderr) - sys.exit(2) - - -def exit_redis_search_error( - args: Namespace, index: SearchIndex | None, exc: RedisSearchError -) -> None: - name = cli_index_target_name(args, index) - print( - f"Redis search operation failed for index {name!r}. {exc}", - file=sys.stderr, - ) - sys.exit(1) From 6322e19282710960b39318674a4dac61b9960454 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Wed, 29 Apr 2026 21:36:13 +0800 Subject: [PATCH 25/36] Revert "feat(cli): add --json to rvl version" This reverts commit ddcd751668cecd21929d4f95f2ab0e68d92eaee4. --- redisvl/cli/version.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/redisvl/cli/version.py b/redisvl/cli/version.py index ec6589c3..13facbcd 100644 --- a/redisvl/cli/version.py +++ b/redisvl/cli/version.py @@ -3,7 +3,6 @@ from argparse import Namespace from redisvl import __version__ -from redisvl.cli.utils import add_json_output_flag, cli_print_json from redisvl.utils.log import get_logger logger = get_logger("[RedisVL]") @@ -22,15 +21,11 @@ def __init__(self): parser.add_argument( "-s", "--short", help="Print only the version number", action="store_true" ) - parser = add_json_output_flag(parser) args = parser.parse_args(sys.argv[2:]) self.version(args) def version(self, args: Namespace): - if args.json: - cli_print_json({"version": __version__}) - return if args.short: print(__version__) else: From 81518d3fbbe3ede3fc3bc0583d3afca0185234a7 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Wed, 29 Apr 2026 21:41:56 +0800 Subject: [PATCH 26/36] reverted tests for JSON output in version.py --- tests/unit/test_cli_utils.py | 3 +- tests/unit/test_cli_version.py | 59 ---------------------------------- 2 files changed, 2 insertions(+), 60 deletions(-) delete mode 100644 tests/unit/test_cli_version.py diff --git a/tests/unit/test_cli_utils.py b/tests/unit/test_cli_utils.py index 970d4684..0007730c 100644 --- a/tests/unit/test_cli_utils.py +++ b/tests/unit/test_cli_utils.py @@ -1,5 +1,6 @@ import json from argparse import ArgumentParser +from typing import Optional import pytest @@ -93,7 +94,7 @@ def test_parser_leaves_connection_options_unset_by_default(parse_args): ], ) def test_create_redis_url_resolves_connection_sources( - parse_args, monkeypatch, argv: list[str], env_url: str | None, expected: str + parse_args, monkeypatch, argv: list[str], env_url: Optional[str], expected: str ): """Resolve Redis URLs from CLI args, environment, and local defaults.""" if env_url is None: diff --git a/tests/unit/test_cli_version.py b/tests/unit/test_cli_version.py deleted file mode 100644 index 18b05d66..00000000 --- a/tests/unit/test_cli_version.py +++ /dev/null @@ -1,59 +0,0 @@ -import json -import sys - -import pytest - -from redisvl import __version__ -from redisvl.cli.version import Version - - -def test_version_json_prints_package_version(monkeypatch, capsys): - """``rvl version --json`` prints only a JSON object with the package version. - - Expected: stdout is a single JSON object with top-level key ``version`` and a - string value equal to ``redisvl.__version__``; no other keys. - """ - monkeypatch.setattr(sys, "argv", ["rvl", "version", "--json"]) - Version() - out = capsys.readouterr().out.strip() - data = json.loads(out) - assert set(data.keys()) == {"version"} - assert data["version"] == __version__ - - -def test_version_short_prints_plain_version(monkeypatch, capsys): - """``rvl version --short`` prints the bare version string (no JSON, no log prefix on stdout). - - Expected: stdout strip equals ``__version__`` only. - """ - monkeypatch.setattr(sys, "argv", ["rvl", "version", "--short"]) - Version() - assert capsys.readouterr().out.strip() == __version__ - - -@pytest.mark.parametrize( - "extra", - [ - pytest.param(["--json", "--short"], id="json-then-short"), - pytest.param(["--short", "--json"], id="short-then-json"), - ], -) -def test_version_json_overrides_short(monkeypatch, capsys, extra): - """If both ``--json`` and ``--short`` are set, JSON output wins regardless of order. - - Expected: same as ``--json`` alone: one JSON object ``{"version": }``. - """ - monkeypatch.setattr(sys, "argv", ["rvl", "version", *extra]) - Version() - out = capsys.readouterr().out.strip() - data = json.loads(out) - assert data == {"version": __version__} - - -def test_version_default_does_not_raise(monkeypatch): - """``rvl version`` with no extra flags runs without raising (default human/log path). - - Expected: ``Version()`` completes; stdout may be empty while the version is logged. - """ - monkeypatch.setattr(sys, "argv", ["rvl", "version"]) - Version() From 7d20ffbe643ceb5e58462ba5cbe86a79b457e1e1 Mon Sep 17 00:00:00 2001 From: ht <121932573+hillarytoh@users.noreply.github.com> Date: Wed, 29 Apr 2026 22:05:28 +0800 Subject: [PATCH 27/36] chore: apply black to stats CLI Made-with: Cursor --- redisvl/cli/stats.py | 1 + 1 file changed, 1 insertion(+) diff --git a/redisvl/cli/stats.py b/redisvl/cli/stats.py index a55beb7d..539aabd3 100644 --- a/redisvl/cli/stats.py +++ b/redisvl/cli/stats.py @@ -43,6 +43,7 @@ def exit_redis_search_error( ) sys.exit(1) + STATS_KEYS = [ "num_docs", "num_terms", From 9295f804c80292e00d1f605fccd93039365143ac Mon Sep 17 00:00:00 2001 From: limjoobin Date: Thu, 30 Apr 2026 03:24:40 +0800 Subject: [PATCH 28/36] chore: Corrected import formatting using isort --- redisvl/cli/index.py | 1 + redisvl/cli/stats.py | 1 + 2 files changed, 2 insertions(+) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index 28c93a12..1a805486 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -1,6 +1,7 @@ import argparse import sys from argparse import Namespace + import yaml from pydantic import ValidationError diff --git a/redisvl/cli/stats.py b/redisvl/cli/stats.py index e1551e9d..e7991d2c 100644 --- a/redisvl/cli/stats.py +++ b/redisvl/cli/stats.py @@ -1,6 +1,7 @@ import argparse import sys from argparse import Namespace + import yaml from pydantic import ValidationError From b3996c33f75a2de386146bbf4d35a704f33ecf16 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Thu, 30 Apr 2026 04:10:44 +0800 Subject: [PATCH 29/36] test(cli): add tests for top-level rvl --help and unknown command paths --- tests/unit/test_cli_main.py | 89 +++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 tests/unit/test_cli_main.py diff --git a/tests/unit/test_cli_main.py b/tests/unit/test_cli_main.py new file mode 100644 index 00000000..0e9c36f2 --- /dev/null +++ b/tests/unit/test_cli_main.py @@ -0,0 +1,89 @@ +import re +import subprocess +import sys + +import pytest + +from redisvl.cli.main import RedisVlCLI + +_COMMANDS = ("index", "mcp", "version", "stats") + + +def _assert_help_contract(help_text: str) -> None: + """Assert key help text users rely on: description, usage, and commands.""" + # Includes the CLI description. + assert "Redis Vector Library CLI" in help_text + # Includes the command section header. + assert "Commands:" in help_text + for name in _COMMANDS: + # Includes each supported top-level command. + assert re.search(rf"^\s*{re.escape(name)}\s+", help_text, re.MULTILINE) + # Includes the short usage form. + assert "rvl []" in help_text + + +@pytest.mark.parametrize("argv", [["rvl"], ["rvl", "--help"], ["rvl", "-h"]]) +def test_rvl_help(monkeypatch, capsys, argv: list[str]): + """Help paths (`rvl`, `--help`, `-h`) exit 0 and print to stdout.""" + + # Arrange + monkeypatch.setattr(sys, "argv", argv) + + # Act + with pytest.raises(SystemExit) as exc_info: + RedisVlCLI() + out = capsys.readouterr() + + # Assert: help requests terminate successfully. + assert exc_info.value.code == 0 + + # Assert: successful help output does not leak to stderr. + assert out.err == "" + + # Assert: stdout contains the expected top-level help contract. + _assert_help_contract(out.out) + + +def test_unknown_command(monkeypatch, capsys): + """Unknown commands exit 2, write error/help to stderr, and keep stdout empty.""" + + # Arrange + monkeypatch.setattr(sys, "argv", ["rvl", "notacommand"]) + + # Act + with pytest.raises(SystemExit) as exc_info: + RedisVlCLI() + out = capsys.readouterr() + + # Assert: unknown commands use the CLI usage-error exit code. + assert exc_info.value.code == 2 + + # Assert: stdout stays empty on this error path. + assert out.out == "" + + # Assert: stderr identifies the rejected command token. + assert "Unknown command: notacommand" in out.err + for name in _COMMANDS: + # Assert: stderr help still lists every valid top-level command. + assert name in out.err + # Assert: stderr includes the command section header from help. + assert "Commands:" in out.err + + +def test_subprocess_module_help(): + """Run ``python -m redisvl.cli.runner --help`` and verify it exits 0 with help on stdout. + + Acts as an end-to-end check that the installed CLI entrypoint actually works. + """ + result = subprocess.run( + [sys.executable, "-m", "redisvl.cli.runner", "--help"], + check=False, + capture_output=True, + text=True, + ) + # Help subprocess exits successfully. + assert result.returncode == 0 + # No stderr output for help. + assert result.stderr == "" + # Stdout includes the same help contract as in-process tests. + _assert_help_contract(result.stdout) \ No newline at end of file From d526875ce92a544d96236a5ed3c7e900068a9a4d Mon Sep 17 00:00:00 2001 From: limjoobin Date: Thu, 30 Apr 2026 04:18:18 +0800 Subject: [PATCH 30/36] chore: Tightened some formatting with inline comments --- tests/unit/test_cli_main.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/tests/unit/test_cli_main.py b/tests/unit/test_cli_main.py index 0e9c36f2..9f91201d 100644 --- a/tests/unit/test_cli_main.py +++ b/tests/unit/test_cli_main.py @@ -25,48 +25,42 @@ def _assert_help_contract(help_text: str) -> None: @pytest.mark.parametrize("argv", [["rvl"], ["rvl", "--help"], ["rvl", "-h"]]) def test_rvl_help(monkeypatch, capsys, argv: list[str]): """Help paths (`rvl`, `--help`, `-h`) exit 0 and print to stdout.""" - - # Arrange monkeypatch.setattr(sys, "argv", argv) - # Act with pytest.raises(SystemExit) as exc_info: RedisVlCLI() out = capsys.readouterr() - # Assert: help requests terminate successfully. + # Help requests terminate successfully. assert exc_info.value.code == 0 - # Assert: successful help output does not leak to stderr. + # Successful help output does not leak to stderr. assert out.err == "" - # Assert: stdout contains the expected top-level help contract. + # stdout contains the expected top-level help contract. _assert_help_contract(out.out) def test_unknown_command(monkeypatch, capsys): """Unknown commands exit 2, write error/help to stderr, and keep stdout empty.""" - - # Arrange monkeypatch.setattr(sys, "argv", ["rvl", "notacommand"]) - # Act with pytest.raises(SystemExit) as exc_info: RedisVlCLI() out = capsys.readouterr() - # Assert: unknown commands use the CLI usage-error exit code. + # Unknown commands use the CLI usage-error exit code. assert exc_info.value.code == 2 - # Assert: stdout stays empty on this error path. + # stdout stays empty on this error path. assert out.out == "" - # Assert: stderr identifies the rejected command token. + # stderr identifies the rejected command token. assert "Unknown command: notacommand" in out.err for name in _COMMANDS: - # Assert: stderr help still lists every valid top-level command. + # stderr help still lists every valid top-level command. assert name in out.err - # Assert: stderr includes the command section header from help. + # stderr includes the command section header from help. assert "Commands:" in out.err From 4db76257d422b4fb8a3bdcb3e4409e4f1c7e6ead Mon Sep 17 00:00:00 2001 From: limjoobin Date: Thu, 30 Apr 2026 21:03:10 +0800 Subject: [PATCH 31/36] test: Added test for CLI contracts related to index --- tests/unit/test_cli_index.py | 643 +++++++++++++++++++++++++++++------ 1 file changed, 541 insertions(+), 102 deletions(-) diff --git a/tests/unit/test_cli_index.py b/tests/unit/test_cli_index.py index 0a3c2f81..16ebe3e6 100644 --- a/tests/unit/test_cli_index.py +++ b/tests/unit/test_cli_index.py @@ -1,10 +1,125 @@ import json +import re +import subprocess import sys import pytest from redisvl.cli.index import Index, _index_info_for_json +_INDEX_SUBCOMMANDS = ("info", "create", "delete", "destroy", "listall") + + +def _assert_index_help_contract(help_text: str) -> None: + """Assert key help text users rely on for ``rvl index``: usage, command header, and subcommands.""" + # Includes the short usage form for the index subcommand router. + assert "rvl index []" in help_text + # Includes the command section header that introduces the subcommand list. + assert "Commands:" in help_text + for name in _INDEX_SUBCOMMANDS: + # Includes each supported index subcommand on its own help line. + assert re.search(rf"^\s*{re.escape(name)}\s+", help_text, re.MULTILINE) + + +@pytest.mark.parametrize("argv", [["rvl", "index", "--help"], ["rvl", "index", "-h"]]) +def test_rvl_index_help(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl index --help`` and ``-h`` are discoverable. + + Expected behavior: ``SystemExit`` code is 0, stdout contains the documented + ``rvl index`` help contract, and stderr is empty. + """ + monkeypatch.setattr(sys, "argv", argv) + + with pytest.raises(SystemExit) as exc_info: + Index() + captured = capsys.readouterr() + + # Help requests terminate successfully. + assert exc_info.value.code == 0 + # Successful help output does not leak to stderr. + assert captured.err == "" + # stdout contains the documented ``rvl index`` help contract. + _assert_index_help_contract(captured.out) + + +@pytest.mark.parametrize("argv", [["rvl", "index"], ["rvl", "index", "--json"]]) +def test_rvl_index_no_subcommand(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl index`` without a subcommand fails with a usage error. + + Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr + is argparse's usage line. The ``--json`` case is parametrized so machine + consumers never see partial JSON when the subcommand is missing. + """ + monkeypatch.setattr(sys, "argv", argv) + + with pytest.raises(SystemExit) as exc_info: + Index() + captured = capsys.readouterr() + + # Argparse usage-error exit code for missing required positional. + assert exc_info.value.code == 2 + # No stdout output - critical so --json consumers do not see partial JSON either. + assert captured.out == "" + # Argparse usage line is rendered to stderr. + assert "usage:" in captured.err.lower() + # Stderr identifies the parser by its prog name (``rvl index``). + assert "rvl index" in captured.err + + +def test_rvl_index_unknown_subcommand(monkeypatch, capsys): + """Tests that ``rvl index `` reports the bad token and prints help. + + Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr + contains ``Unknown command: `` followed by the full help text. + """ + monkeypatch.setattr(sys, "argv", ["rvl", "index", "notacommand"]) + + with pytest.raises(SystemExit) as exc_info: + Index() + captured = capsys.readouterr() + + # Documented usage-error exit for an unknown subcommand. + assert exc_info.value.code == 2 + # No normal output for the invalid subcommand path. + assert captured.out == "" + # Stderr identifies the rejected subcommand token. + assert "Unknown command: notacommand" in captured.err + for name in _INDEX_SUBCOMMANDS: + # Stderr help still lists every valid index subcommand. + assert name in captured.err + # Stderr help includes the command section header from the help text. + assert "Commands:" in captured.err + + +def test_rvl_index_subprocess_help(): + """End-to-end smoke test of ``rvl index --help`` via the runner module. + + Expected behavior: the subprocess exits 0, stdout matches the same help + contract as the in-process test, and stderr is empty - confirming the + installed entrypoint wires through to ``Index`` correctly. + """ + result = subprocess.run( + [sys.executable, "-m", "redisvl.cli.runner", "index", "--help"], + check=False, + capture_output=True, + text=True, + ) + # Process exited cleanly. + assert result.returncode == 0 + # Help is not emitted on stderr. + assert result.stderr == "" + # stdout includes the same help contract as the in-process help test. + _assert_index_help_contract(result.stdout) + + +def _raise(exc: BaseException): + """Return a zero-arg callable that raises ``exc``.""" + + def _do(): + raise exc + + return _do + class _FakeConn: def __init__(self, result, boom=False): @@ -12,97 +127,335 @@ def __init__(self, result, boom=False): self._boom = boom def execute_command(self, cmd): - assert cmd == "FT._LIST" # listall must query Redis with FT._LIST + # listall must query Redis with FT._LIST + assert cmd == "FT._LIST" if self._boom: raise RuntimeError("redis unavailable") return self._result -def test_listall_json(monkeypatch, capsys): - """Tests that ``listall --json`` prints machine-readable output only. +def _patch_redis_connection(monkeypatch, *, result=None, boom: bool = False) -> None: + """Patch ``RedisConnectionFactory.get_redis_connection`` to return a ``_FakeConn``. - Expected behavior: stdout is one JSON line with ``indices`` in order and no table text. + ``result`` is what ``execute_command("FT._LIST")`` returns on the success + path (defaults to ``[]``); ``boom=True`` makes ``execute_command`` raise a + ``RuntimeError`` instead, exercising the runtime-failure branch. """ + fake = _FakeConn([] if result is None else result, boom=boom) + monkeypatch.setattr( + "redisvl.cli.index.RedisConnectionFactory.get_redis_connection", + lambda *a, **k: fake, + ) + + +def _patch_search_index( + monkeypatch, + *, + create_behavior=None, + from_yaml_raises: BaseException | None = None, + index_name: str = "test-idx", +) -> None: + """Patch ``redisvl.cli.index.SearchIndex`` with a minimal fake for ``rvl index create`` tests. + + ``from_yaml_raises`` makes ``from_yaml`` raise that exception; ``create_behavior`` + is invoked inside ``FakeIndex.create()`` (use :func:`_raise` for failures); + ``index_name`` is surfaced via ``index.schema.index.name``. + """ + if from_yaml_raises is not None: + + class FakeSearchIndex: + @classmethod + def from_yaml(cls, *_args, **_kwargs): + raise from_yaml_raises + + else: + + class _FakeIndexInfo: + name = index_name + + class _FakeSchema: + index = _FakeIndexInfo() - def fake_get(*a, **k): - return _FakeConn([b"idx_a", b"idx_b"]) + class FakeIndex: + schema = _FakeSchema() + def create(self): + if create_behavior is not None: + create_behavior() + + class FakeSearchIndex: + @classmethod + def from_yaml(cls, *_args, **_kwargs): + return FakeIndex() + + monkeypatch.setattr("redisvl.cli.index.SearchIndex", FakeSearchIndex) + + +def _patch_search_index_for_info( + monkeypatch, + *, + info_behavior=None, + with_field_options: bool = False, + index_name: str = "test-idx", +) -> None: + """Patch ``redisvl.cli.index.SearchIndex`` with a minimal fake for ``rvl index info`` tests. + + By default builds a happy-path FakeIndex whose ``info()`` returns a fixed + FT.INFO payload (with an extra ``NOSTEM=1`` attribute when + ``with_field_options=True``). Pass ``info_behavior=_raise(SomeError())`` + to make ``info()`` raise instead. ``index_name`` is surfaced via + ``index.schema.index.name`` so ``exit_redis_search_error`` can format its + message verbatim. + """ + if info_behavior is None: + attrs = [b"identifier", b"u", b"attribute", b"u", b"type", b"TAG"] + if with_field_options: + attrs.extend([b"NOSTEM", b"1"]) + payload = { + "index_name": b"test-idx", + "index_definition": ["key_type", b"HASH", "prefixes", [b"pre"]], + "attributes": [attrs], + } + + def info_behavior(): + return payload + + class _FakeIndexInfo: + name = index_name + + class _FakeSchema: + index = _FakeIndexInfo() + + class FakeIndex: + schema = _FakeSchema() + + def __init__(self, *a, **k): + # Absorb the ``schema=`` and ``redis_url=`` kwargs that + # ``_connect_to_index`` passes to ``SearchIndex(...)`` directly. + # Without this, Python's default ``__init__`` rejects them. + pass + + def info(self): + return info_behavior() + + monkeypatch.setattr("redisvl.cli.index.SearchIndex", FakeIndex) + + +def test_create_missing_schema(monkeypatch, capsys): + """Tests that ``rvl index create`` without ``-s`` exits with the missing-schema usage error. + + Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr + is exactly ``Schema must be provided to create an index\\n``. + """ + monkeypatch.setattr(sys, "argv", ["rvl", "index", "create"]) + + with pytest.raises(SystemExit) as exc_info: + Index() + captured = capsys.readouterr() + + # Documented usage-error exit when -s is not provided. + assert exc_info.value.code == 2 + # Nothing leaks on this error path. + assert captured.out == "" + # Exact stderr contract from the missing-schema guard's print() call. + assert captured.err == "Schema must be provided to create an index\n" + + +def test_create_schema_input_error(monkeypatch, capsys): + """Tests that ``rvl index create`` reports schema-load failures on stderr. + + Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr + is exactly the raised exception's message followed by a newline. + """ + schema_error_message = "schema file missing: /does/not/exist.yaml" + + _patch_search_index( + monkeypatch, from_yaml_raises=FileNotFoundError(schema_error_message) + ) monkeypatch.setattr( - "redisvl.cli.index.RedisConnectionFactory.get_redis_connection", fake_get + sys, "argv", ["rvl", "index", "create", "-s", "/does/not/exist.yaml"] ) - monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) - Index() - out = capsys.readouterr().out.strip() - assert "Indices:" not in out # --json must not print the human banner - assert out.count("\n") == 0 # single machine-readable line, nothing else on stdout - payload = json.loads(out) - assert payload == {"indices": ["idx_a", "idx_b"]} # same order/encoding as table path would show + with pytest.raises(SystemExit) as exc_info: + Index() + captured = capsys.readouterr() + + # exit_schema_input_error uses exit code 2 when -s was provided. + assert exc_info.value.code == 2 + # Nothing on stdout when schema input fails. + assert captured.out == "" + # Exact stderr contract: exit_schema_input_error does print(str(exc), file=sys.stderr). + assert captured.err == f"{schema_error_message}\n" -def test_listall_table(monkeypatch, capsys): - """Tests that default ``listall`` keeps the human-readable table output. - Expected behavior: stdout matches header + numbered rows in FT._LIST order. +def test_create_redis_search_error(monkeypatch, capsys): + """Tests that ``rvl index create`` reports Redis-side failures on stderr. + + Expected behavior: ``SystemExit`` code is 1, stdout is empty, and stderr + contains both the ``Redis search operation failed for index 'test-idx'.`` + prefix and the underlying error message. """ + from redisvl.exceptions import RedisSearchError - def fake_get(*a, **k): - return _FakeConn([b"one", b"two"]) + redis_error_message = "create failed" - monkeypatch.setattr( - "redisvl.cli.index.RedisConnectionFactory.get_redis_connection", fake_get + _patch_search_index( + monkeypatch, + create_behavior=_raise(RedisSearchError(redis_error_message)), + index_name="test-idx", ) - monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall"]) - Index() - out = capsys.readouterr().out - lines = [ln.strip() for ln in out.strip().splitlines()] - assert lines == [ - "Indices:", - "1. one", - "2. two", - ] # exact table output: header then rows matching mock order and labels + monkeypatch.setattr(sys, "argv", ["rvl", "index", "create", "-s", "fake.yaml"]) + with pytest.raises(SystemExit) as exc_info: + Index() + captured = capsys.readouterr() + + # Documented Redis-failure exit code from exit_redis_search_error. + assert exc_info.value.code == 1 + # No partial output before the failure. + assert captured.out == "" + # Documented error prefix, with the index name surfacing verbatim. + assert "Redis search operation failed for index 'test-idx'." in captured.err + # The exception's str() is forwarded to stderr verbatim. + assert redis_error_message in captured.err -def test_listall_json_empty(monkeypatch, capsys): - """Tests that ``listall --json`` handles an empty FT._LIST result. - Expected behavior: stdout is valid JSON with ``{"indices": []}``. +def test_create_success(monkeypatch, capsys): + """Tests that ``rvl index create`` succeeds with the documented banner. + + Expected behavior: no ``SystemExit`` is raised, stdout is exactly + ``Index created successfully\\n``, and stderr is empty. """ + _patch_search_index(monkeypatch) + monkeypatch.setattr(sys, "argv", ["rvl", "index", "create", "-s", "fake.yaml"]) + + # Success path must return cleanly, not raise SystemExit. + Index() + captured = capsys.readouterr() + + # Exact stdout: the success banner with print()'s trailing newline and nothing else. + assert captured.out == "Index created successfully\n" + # Success path stays clean on stderr. + assert captured.err == "" + - def fake_get(*a, **k): - return _FakeConn([]) +def test_create_json_flag_no_json(monkeypatch, capsys): + """Tests that ``rvl index create --json`` does not invent a JSON contract. + Expected behavior: stdout matches the no-flag success banner exactly and + stderr is empty - ``--json`` is a no-op for ``create``. + """ + _patch_search_index(monkeypatch) monkeypatch.setattr( - "redisvl.cli.index.RedisConnectionFactory.get_redis_connection", fake_get + sys, + "argv", + ["rvl", "index", "create", "-s", "fake.yaml", "--json"], ) + + # Success path must return cleanly, not call sys.exit, even with --json. + Index() + captured = capsys.readouterr() + + # Success path stays clean on stderr even with --json. + assert captured.err == "" + + # stdout is identical to the no-flag success path, proving --json invents no JSON contract. + assert captured.out == "Index created successfully\n" + + +def test_listall_json(monkeypatch, capsys): + """Tests that ``rvl index listall --json`` prints the documented JSON contract. + + Expected behavior: no ``SystemExit`` is raised, stdout is one JSON line + of the form ``{"indices": [...]}`` (no human banner), and stderr is empty. + """ + _patch_redis_connection(monkeypatch, result=[b"idx_a", b"idx_b"]) monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) + + try: + Index() + except SystemExit as exc: + # Success path must return cleanly, not call sys.exit. + pytest.fail(f"listall --json raised SystemExit({exc.code}) on the success path") + captured = capsys.readouterr() + out = captured.out.strip() + + # --json must not print the human banner. + assert "Indices:" not in out + # Single machine-readable line, nothing else on stdout. + assert out.count("\n") == 0 + # Stable top-level JSON contract: only the "indices" key, in FT._LIST order. + assert json.loads(out) == {"indices": ["idx_a", "idx_b"]} + # JSON success path is silent on stderr. + assert captured.err == "" + + +def test_listall_table(monkeypatch, capsys): + """Tests that default ``rvl index listall`` prints the human-readable table. + + Expected behavior: stdout is exactly ``Indices:\\n1. one\\n2. two\\n`` + (header + 1-indexed rows in FT._LIST order), and stderr is empty. + """ + _patch_redis_connection(monkeypatch, result=[b"one", b"two"]) + monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall"]) Index() - out = capsys.readouterr().out.strip() - assert json.loads(out) == {"indices": []} # empty array is a valid success payload + captured = capsys.readouterr() + # Exact stdout: header + numbered rows in FT._LIST order, each from a single print(). + assert captured.out == "Indices:\n1. one\n2. two\n" + # Table success is silent on stderr. + assert captured.err == "" -def test_listall_json_error(monkeypatch, capsys): - """Tests that ``listall --json`` failure exits cleanly without stdout JSON. - Expected behavior: ``SystemExit`` code is 0 and stdout is empty. +def test_listall_json_empty(monkeypatch, capsys): + """Tests that ``rvl index listall --json`` handles an empty FT._LIST result. + + Expected behavior: stdout is one JSON line with ``{"indices": []}`` (no + human banner), and stderr is empty. """ + _patch_redis_connection(monkeypatch) + monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) - def fake_get(*a, **k): - return _FakeConn([], boom=True) + Index() + captured = capsys.readouterr() + out = captured.out.strip() - monkeypatch.setattr( - "redisvl.cli.index.RedisConnectionFactory.get_redis_connection", fake_get - ) + # Empty --json must not print the human banner. + assert "Indices:" not in out + # Single machine-readable line for the empty case too. + assert out.count("\n") == 0 + # Empty array is a valid success payload. + assert json.loads(out) == {"indices": []} + # JSON success path is silent on stderr. + assert captured.err == "" + + +def test_listall_json_error(monkeypatch, capsys): + """Tests that ``rvl index listall --json`` reports runtime failures on stderr. + + Expected behavior: ``SystemExit`` code is 1, stdout is empty (no + half-formed JSON), and the underlying error message is on stderr. + """ + _patch_redis_connection(monkeypatch, boom=True) monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) - with pytest.raises(SystemExit) as excinfo: # exit(0) in Index.__init__ is not a plain return + + with pytest.raises(SystemExit) as exc_info: Index() - assert excinfo.value.code == 0 # "log and exit(0)" CLI contract - assert capsys.readouterr().out == "" # failure before cli_print_json — nothing on stdout + captured = capsys.readouterr() + + # generic runtime failure exits 1 from __init__'s catch-all + assert exc_info.value.code == 1 + # failure happens before cli_print_json — nothing on stdout + assert captured.out == "" + # underlying error message is surfaced on stderr + assert "redis unavailable" in captured.err def test_info_json_normalize(): """Tests that ``_index_info_for_json`` maps FT.INFO lists to structured JSON. - Expected behavior: input is unchanged and output has ``index_information`` + ``index_fields``. + Expected behavior: the input dict is not mutated and the output has the + documented top-level keys ``index_information`` and ``index_fields``. """ raw = { "index_name": "test_index", @@ -125,7 +478,10 @@ def test_info_json_normalize(): } before = str(raw) out = _index_info_for_json(raw) - assert str(raw) == before # not mutated + + # Helper does not mutate its input. + assert str(raw) == before + # Exact summary + fields payload, matching what the table prints semantically. assert out == { "index_information": { "index_name": "test_index", @@ -141,13 +497,15 @@ def test_info_json_normalize(): "type": "TAG", } ], - } # exact summary+fields payload, matching what table prints semantically + } def test_info_json(monkeypatch, capsys): - """Tests that ``info --json`` returns normalized table-equivalent JSON. + """Tests that ``rvl index info --json`` prints the documented JSON contract. - Expected behavior: one parseable JSON line with decoded values and no table banners. + Expected behavior: no ``SystemExit`` is raised, stdout is one parseable + JSON line with the documented top-level sections (no table banners), + and stderr is empty. """ expected_index_information = { @@ -164,64 +522,145 @@ def test_info_json(monkeypatch, capsys): "field_options": {"NOSTEM": "1"}, } - class FakeIndex: - def __init__(self, *a, **k): - pass - - def info(self): - return { - "index_name": b"test-idx", - "index_definition": [ - "key_type", - b"HASH", - "prefixes", - [b"pre"], - ], - "attributes": [ - [ - b"identifier", - b"u", - b"attribute", - b"u", - b"type", - b"TAG", - b"NOSTEM", - b"1", - ], - ], - } - - monkeypatch.setattr("redisvl.cli.index.SearchIndex", FakeIndex) + _patch_search_index_for_info(monkeypatch, with_field_options=True) monkeypatch.setattr( sys, "argv", ["rvl", "index", "info", "-i", "test-idx", "--json"] ) - Index() - out = capsys.readouterr().out.strip() - assert out.count("\n") == 0 # single line for machine consumers + + try: + Index() + except SystemExit as exc: + # Success path must return cleanly, not call sys.exit. + pytest.fail(f"info --json raised SystemExit({exc.code}) on the success path") + captured = capsys.readouterr() + out = captured.out.strip() + + # Single line for machine consumers. + assert out.count("\n") == 0 payload = json.loads(out) - assert "Index Information:" not in out and "Index Fields:" not in out # --json must not emit table banner text - assert list(payload) == ["index_information", "index_fields"] # top-level sections are stable and ordered - assert payload["index_information"] == expected_index_information # summary section matches table-derived values - assert payload["index_fields"] == [expected_field] # one normalized field row with options + # --json must not emit table banner text. + assert "Index Information:" not in out and "Index Fields:" not in out + # Top-level sections are stable and ordered. + assert list(payload) == ["index_information", "index_fields"] + # Summary section matches table-derived values. + assert payload["index_information"] == expected_index_information + # One normalized field row with options. + assert payload["index_fields"] == [expected_field] + # JSON success path is silent on stderr. + assert captured.err == "" + def test_info_json_error(monkeypatch, capsys): - """Tests that ``info --json`` errors do not emit partial stdout JSON. + """Tests that ``rvl index info --json`` reports runtime failures on stderr. - Expected behavior: command exits with code 0 and stdout is empty. + Expected behavior: ``SystemExit`` code is 1, stdout is empty (no + half-formed JSON), and the underlying error message is on stderr. """ + _patch_search_index_for_info( + monkeypatch, info_behavior=_raise(RuntimeError("boom")) + ) + monkeypatch.setattr( + sys, "argv", ["rvl", "index", "info", "-i", "test-idx", "--json"] + ) - class BoomIndex: - def __init__(self, *a, **k): - pass + with pytest.raises(SystemExit) as exc_info: + Index() + captured = capsys.readouterr() + + # Generic runtime failure exits 1 from __init__'s catch-all. + assert exc_info.value.code == 1 + # No partial JSON before the exception. + assert captured.out == "" + # Underlying error message is surfaced on stderr. + assert "boom" in captured.err + + +@pytest.mark.parametrize( + "argv", + [ + ["rvl", "index", "info"], + ["rvl", "index", "info", "--json"], + ], +) +def test_info_no_target(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl index info`` without ``-i`` or ``-s`` exits with the usage error. + + Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr + is exactly ``Index name or schema must be provided\\n``. ``--json`` is + parametrized to confirm no JSON contract is invented for usage errors. + """ + monkeypatch.setattr(sys, "argv", argv) - def info(self): - raise RuntimeError("boom") + with pytest.raises(SystemExit) as exc_info: + Index() + captured = capsys.readouterr() - monkeypatch.setattr("redisvl.cli.index.SearchIndex", BoomIndex) - monkeypatch.setattr( - sys, "argv", ["rvl", "index", "info", "-i", "test-idx", "--json"] + # _connect_to_index uses argparse-style usage exit code + assert exc_info.value.code == 2 + # usage errors must not pollute stdout, even with --json + assert captured.out == "" + # exact stderr message contract from _connect_to_index + assert captured.err == "Index name or schema must be provided\n" + + +def test_info_table(monkeypatch, capsys): + """Tests that default ``rvl index info`` prints the human-readable table. + + Expected behavior: stdout includes the ``Index Information:`` and + ``Index Fields:`` banners and surfaces the index name and storage type; + stderr is empty. + """ + _patch_search_index_for_info(monkeypatch) + monkeypatch.setattr(sys, "argv", ["rvl", "index", "info", "-i", "test-idx"]) + + Index() + captured = capsys.readouterr() + + # Table success is silent on stderr. + assert captured.err == "" + # Top-of-table banner is printed. + assert "Index Information:" in captured.out + # Fields-table banner is printed. + assert "Index Fields:" in captured.out + # The index name surfaces in the rendered cells. + assert "test-idx" in captured.out + # The storage type surfaces in the rendered cells. + assert "HASH" in captured.out + + +@pytest.mark.parametrize( + "argv", + [ + ["rvl", "index", "info", "-i", "test-idx"], + ["rvl", "index", "info", "-i", "test-idx", "--json"], + ], +) +def test_info_missing_index(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl index info -i `` reports the missing index on stderr. + + Expected behavior: ``SystemExit`` code is 1, stdout is empty, and stderr + contains both the ``Redis search operation failed for index 'test-idx'.`` + prefix and the underlying error message. ``--json`` is parametrized to + confirm the contract does not change. + """ + from redisvl.exceptions import RedisSearchError + + underlying_error = "Unknown index name" + + _patch_search_index_for_info( + monkeypatch, info_behavior=_raise(RedisSearchError(underlying_error)) ) - with pytest.raises(SystemExit) as excinfo: + monkeypatch.setattr(sys, "argv", argv) + + with pytest.raises(SystemExit) as exc_info: Index() - assert excinfo.value.code == 0 # try/except in Index.__init__ + exit(0) - assert capsys.readouterr().out == "" # no partial JSON before the exception + captured = capsys.readouterr() + + # Documented exit code for a Redis search-side failure. + assert exc_info.value.code == 1 + # Nothing leaks on this error path. + assert captured.out == "" + # Documented error prefix, with the index name surfacing verbatim. + assert "Redis search operation failed for index 'test-idx'." in captured.err + # The exception's str() is forwarded to stderr verbatim. + assert underlying_error in captured.err From 940be2d1d9a1c6109d2855ff8f4adf0f87fd0122 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Fri, 1 May 2026 02:01:34 +0800 Subject: [PATCH 32/36] fix: Fixed exception catching for index and stats --- redisvl/cli/index.py | 11 ++++++----- redisvl/cli/stats.py | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/redisvl/cli/index.py b/redisvl/cli/index.py index 1a805486..621e765f 100644 --- a/redisvl/cli/index.py +++ b/redisvl/cli/index.py @@ -114,14 +114,15 @@ def info(self, args: Namespace): rvl index info -i | -s [--json] """ index = self._connect_to_index(args) - index_info = index.info() + try: + index_info = index.info() + except RedisSearchError as e: + exit_redis_search_error(args, index, e) + if args.json: cli_print_json(_index_info_for_json(index_info)) else: - try: - _display_in_table(index_info) - except RedisSearchError as e: - exit_redis_search_error(args, index, e) + _display_in_table(index_info) def listall(self, args: Namespace): """List all indices. diff --git a/redisvl/cli/stats.py b/redisvl/cli/stats.py index e7991d2c..bfa1c668 100644 --- a/redisvl/cli/stats.py +++ b/redisvl/cli/stats.py @@ -110,15 +110,16 @@ def stats(self, args: Namespace): rvl stats -i | -s """ index = self._connect_to_index(args) - index_info = index.info() + try: + index_info = index.info() + except RedisSearchError as e: + exit_redis_search_error(args, index, e) + rows = _stats_rows(index_info) if args.json: cli_print_json(dict(rows)) else: - try: - _display_stats(rows) - except RedisSearchError as e: - exit_redis_search_error(args, index, e) + _display_stats(rows) def _connect_to_index(self, args: Namespace) -> SearchIndex: redis_url = create_redis_url(args) From c9d319bd88cd0aaea9292972381c88c8c4f24ae2 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Tue, 5 May 2026 15:44:46 +0800 Subject: [PATCH 33/36] test: Tightened the test cases --- tests/unit/test_cli_index.py | 86 ++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 47 deletions(-) diff --git a/tests/unit/test_cli_index.py b/tests/unit/test_cli_index.py index 16ebe3e6..a30dda4e 100644 --- a/tests/unit/test_cli_index.py +++ b/tests/unit/test_cli_index.py @@ -320,14 +320,22 @@ def test_create_redis_search_error(monkeypatch, capsys): assert redis_error_message in captured.err -def test_create_success(monkeypatch, capsys): +@pytest.mark.parametrize( + "argv", + [ + ["rvl", "index", "create", "-s", "fake.yaml"], + ["rvl", "index", "create", "-s", "fake.yaml", "--json"], + ], +) +def test_create_success(monkeypatch, capsys, argv: list[str]): """Tests that ``rvl index create`` succeeds with the documented banner. Expected behavior: no ``SystemExit`` is raised, stdout is exactly - ``Index created successfully\\n``, and stderr is empty. + ``Index created successfully\\n``, and stderr is empty. ``--json`` is + parametrized to confirm it does not invent a JSON contract for ``create``. """ _patch_search_index(monkeypatch) - monkeypatch.setattr(sys, "argv", ["rvl", "index", "create", "-s", "fake.yaml"]) + monkeypatch.setattr(sys, "argv", argv) # Success path must return cleanly, not raise SystemExit. Index() @@ -339,30 +347,6 @@ def test_create_success(monkeypatch, capsys): assert captured.err == "" -def test_create_json_flag_no_json(monkeypatch, capsys): - """Tests that ``rvl index create --json`` does not invent a JSON contract. - - Expected behavior: stdout matches the no-flag success banner exactly and - stderr is empty - ``--json`` is a no-op for ``create``. - """ - _patch_search_index(monkeypatch) - monkeypatch.setattr( - sys, - "argv", - ["rvl", "index", "create", "-s", "fake.yaml", "--json"], - ) - - # Success path must return cleanly, not call sys.exit, even with --json. - Index() - captured = capsys.readouterr() - - # Success path stays clean on stderr even with --json. - assert captured.err == "" - - # stdout is identical to the no-flag success path, proving --json invents no JSON contract. - assert captured.out == "Index created successfully\n" - - def test_listall_json(monkeypatch, capsys): """Tests that ``rvl index listall --json`` prints the documented JSON contract. @@ -380,8 +364,6 @@ def test_listall_json(monkeypatch, capsys): captured = capsys.readouterr() out = captured.out.strip() - # --json must not print the human banner. - assert "Indices:" not in out # Single machine-readable line, nothing else on stdout. assert out.count("\n") == 0 # Stable top-level JSON contract: only the "indices" key, in FT._LIST order. @@ -420,8 +402,6 @@ def test_listall_json_empty(monkeypatch, capsys): captured = capsys.readouterr() out = captured.out.strip() - # Empty --json must not print the human banner. - assert "Indices:" not in out # Single machine-readable line for the empty case too. assert out.count("\n") == 0 # Empty array is a valid success payload. @@ -430,24 +410,32 @@ def test_listall_json_empty(monkeypatch, capsys): assert captured.err == "" -def test_listall_json_error(monkeypatch, capsys): - """Tests that ``rvl index listall --json`` reports runtime failures on stderr. +@pytest.mark.parametrize( + "argv", + [ + ["rvl", "index", "listall"], + ["rvl", "index", "listall", "--json"], + ], +) +def test_listall_runtime_error(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl index listall`` reports runtime failures on stderr. Expected behavior: ``SystemExit`` code is 1, stdout is empty (no - half-formed JSON), and the underlying error message is on stderr. + half-formed output), and the underlying error message is on stderr. + ``--json`` is parametrized to confirm the contract is uniform. """ _patch_redis_connection(monkeypatch, boom=True) - monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) + monkeypatch.setattr(sys, "argv", argv) with pytest.raises(SystemExit) as exc_info: Index() captured = capsys.readouterr() - # generic runtime failure exits 1 from __init__'s catch-all + # Generic runtime failure exits 1 from __init__'s catch-all. assert exc_info.value.code == 1 - # failure happens before cli_print_json — nothing on stdout + # Failure happens before any rendering - nothing on stdout. assert captured.out == "" - # underlying error message is surfaced on stderr + # Underlying error message is surfaced on stderr. assert "redis unavailable" in captured.err @@ -538,8 +526,6 @@ def test_info_json(monkeypatch, capsys): # Single line for machine consumers. assert out.count("\n") == 0 payload = json.loads(out) - # --json must not emit table banner text. - assert "Index Information:" not in out and "Index Fields:" not in out # Top-level sections are stable and ordered. assert list(payload) == ["index_information", "index_fields"] # Summary section matches table-derived values. @@ -550,18 +536,24 @@ def test_info_json(monkeypatch, capsys): assert captured.err == "" -def test_info_json_error(monkeypatch, capsys): - """Tests that ``rvl index info --json`` reports runtime failures on stderr. +@pytest.mark.parametrize( + "argv", + [ + ["rvl", "index", "info", "-i", "test-idx"], + ["rvl", "index", "info", "-i", "test-idx", "--json"], + ], +) +def test_info_runtime_error(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl index info`` reports runtime failures on stderr. Expected behavior: ``SystemExit`` code is 1, stdout is empty (no - half-formed JSON), and the underlying error message is on stderr. + half-formed output), and the underlying error message is on stderr. + ``--json`` is parametrized to confirm the contract is uniform. """ _patch_search_index_for_info( monkeypatch, info_behavior=_raise(RuntimeError("boom")) ) - monkeypatch.setattr( - sys, "argv", ["rvl", "index", "info", "-i", "test-idx", "--json"] - ) + monkeypatch.setattr(sys, "argv", argv) with pytest.raises(SystemExit) as exc_info: Index() @@ -569,7 +561,7 @@ def test_info_json_error(monkeypatch, capsys): # Generic runtime failure exits 1 from __init__'s catch-all. assert exc_info.value.code == 1 - # No partial JSON before the exception. + # Failure happens before any rendering - nothing on stdout. assert captured.out == "" # Underlying error message is surfaced on stderr. assert "boom" in captured.err From 4410ebae5e05d87d94c065bf5de898e2729f6c7b Mon Sep 17 00:00:00 2001 From: limjoobin Date: Tue, 5 May 2026 15:59:41 +0800 Subject: [PATCH 34/36] test: Modified test to match updated argparse outputs --- tests/unit/test_cli_index.py | 17 ++++++++--------- tests/unit/test_cli_main.py | 6 +++--- tests/unit/test_cli_utils.py | 7 ++++++- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/tests/unit/test_cli_index.py b/tests/unit/test_cli_index.py index a30dda4e..adc58835 100644 --- a/tests/unit/test_cli_index.py +++ b/tests/unit/test_cli_index.py @@ -12,8 +12,8 @@ def _assert_index_help_contract(help_text: str) -> None: """Assert key help text users rely on for ``rvl index``: usage, command header, and subcommands.""" - # Includes the short usage form for the index subcommand router. - assert "rvl index []" in help_text + # Includes argparse's auto-generated usage line for the index subcommand router. + assert "usage: rvl index" in help_text # Includes the command section header that introduces the subcommand list. assert "Commands:" in help_text for name in _INDEX_SUBCOMMANDS: @@ -67,10 +67,11 @@ def test_rvl_index_no_subcommand(monkeypatch, capsys, argv: list[str]): def test_rvl_index_unknown_subcommand(monkeypatch, capsys): - """Tests that ``rvl index `` reports the bad token and prints help. + """Tests that ``rvl index `` reports the bad token via argparse. Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr - contains ``Unknown command: `` followed by the full help text. + contains argparse's ``invalid choice: ''`` error along with the + list of valid subcommands. """ monkeypatch.setattr(sys, "argv", ["rvl", "index", "notacommand"]) @@ -82,13 +83,11 @@ def test_rvl_index_unknown_subcommand(monkeypatch, capsys): assert exc_info.value.code == 2 # No normal output for the invalid subcommand path. assert captured.out == "" - # Stderr identifies the rejected subcommand token. - assert "Unknown command: notacommand" in captured.err + # Stderr identifies the rejected subcommand token via argparse's error. + assert "invalid choice: 'notacommand'" in captured.err for name in _INDEX_SUBCOMMANDS: - # Stderr help still lists every valid index subcommand. + # Stderr lists every valid subcommand. assert name in captured.err - # Stderr help includes the command section header from the help text. - assert "Commands:" in captured.err def test_rvl_index_subprocess_help(): diff --git a/tests/unit/test_cli_main.py b/tests/unit/test_cli_main.py index 9f91201d..d87c9caa 100644 --- a/tests/unit/test_cli_main.py +++ b/tests/unit/test_cli_main.py @@ -14,7 +14,7 @@ def _assert_help_contract(help_text: str) -> None: # Includes the CLI description. assert "Redis Vector Library CLI" in help_text # Includes the command section header. - assert "Commands:" in help_text + assert "Command groups:" in help_text for name in _COMMANDS: # Includes each supported top-level command. assert re.search(rf"^\s*{re.escape(name)}\s+", help_text, re.MULTILINE) @@ -61,7 +61,7 @@ def test_unknown_command(monkeypatch, capsys): # stderr help still lists every valid top-level command. assert name in out.err # stderr includes the command section header from help. - assert "Commands:" in out.err + assert "Command groups:" in out.err def test_subprocess_module_help(): @@ -80,4 +80,4 @@ def test_subprocess_module_help(): # No stderr output for help. assert result.stderr == "" # Stdout includes the same help contract as in-process tests. - _assert_help_contract(result.stdout) \ No newline at end of file + _assert_help_contract(result.stdout) diff --git a/tests/unit/test_cli_utils.py b/tests/unit/test_cli_utils.py index 0007730c..8d80d6e6 100644 --- a/tests/unit/test_cli_utils.py +++ b/tests/unit/test_cli_utils.py @@ -4,7 +4,12 @@ import pytest -from redisvl.cli.utils import add_index_parsing_options, add_json_output_flag, cli_print_json, create_redis_url +from redisvl.cli.utils import ( + add_index_parsing_options, + add_json_output_flag, + cli_print_json, + create_redis_url, +) @pytest.fixture From 264494217fbeef0d1286bd92a6bdb147c1b4dad4 Mon Sep 17 00:00:00 2001 From: limjoobin Date: Tue, 5 May 2026 18:28:39 +0800 Subject: [PATCH 35/36] test: Loosened tests to capture actual expected behaviour and error codes rather than soft asserts --- tests/unit/test_cli_index.py | 136 +++++++++++++---------------------- tests/unit/test_cli_main.py | 12 +--- 2 files changed, 52 insertions(+), 96 deletions(-) diff --git a/tests/unit/test_cli_index.py b/tests/unit/test_cli_index.py index adc58835..2e25e9ea 100644 --- a/tests/unit/test_cli_index.py +++ b/tests/unit/test_cli_index.py @@ -11,13 +11,9 @@ def _assert_index_help_contract(help_text: str) -> None: - """Assert key help text users rely on for ``rvl index``: usage, command header, and subcommands.""" - # Includes argparse's auto-generated usage line for the index subcommand router. - assert "usage: rvl index" in help_text - # Includes the command section header that introduces the subcommand list. - assert "Commands:" in help_text + """Assert that ``rvl index`` help lists every supported subcommand.""" for name in _INDEX_SUBCOMMANDS: - # Includes each supported index subcommand on its own help line. + # Each supported index subcommand appears on its own help line. assert re.search(rf"^\s*{re.escape(name)}\s+", help_text, re.MULTILINE) @@ -47,8 +43,8 @@ def test_rvl_index_no_subcommand(monkeypatch, capsys, argv: list[str]): """Tests that ``rvl index`` without a subcommand fails with a usage error. Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr - is argparse's usage line. The ``--json`` case is parametrized so machine - consumers never see partial JSON when the subcommand is missing. + is non-empty. The ``--json`` case is parametrized so machine consumers + never see partial JSON when the subcommand is missing. """ monkeypatch.setattr(sys, "argv", argv) @@ -60,18 +56,15 @@ def test_rvl_index_no_subcommand(monkeypatch, capsys, argv: list[str]): assert exc_info.value.code == 2 # No stdout output - critical so --json consumers do not see partial JSON either. assert captured.out == "" - # Argparse usage line is rendered to stderr. - assert "usage:" in captured.err.lower() - # Stderr identifies the parser by its prog name (``rvl index``). - assert "rvl index" in captured.err + # Argparse emits a usage error on stderr + assert captured.err != "" def test_rvl_index_unknown_subcommand(monkeypatch, capsys): """Tests that ``rvl index `` reports the bad token via argparse. Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr - contains argparse's ``invalid choice: ''`` error along with the - list of valid subcommands. + lists every valid subcommands. """ monkeypatch.setattr(sys, "argv", ["rvl", "index", "notacommand"]) @@ -83,8 +76,6 @@ def test_rvl_index_unknown_subcommand(monkeypatch, capsys): assert exc_info.value.code == 2 # No normal output for the invalid subcommand path. assert captured.out == "" - # Stderr identifies the rejected subcommand token via argparse's error. - assert "invalid choice: 'notacommand'" in captured.err for name in _INDEX_SUBCOMMANDS: # Stderr lists every valid subcommand. assert name in captured.err @@ -241,10 +232,10 @@ def info(self): def test_create_missing_schema(monkeypatch, capsys): - """Tests that ``rvl index create`` without ``-s`` exits with the missing-schema usage error. + """Tests that ``rvl index create`` without ``-s`` exits with a usage error. Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr - is exactly ``Schema must be provided to create an index\\n``. + is non-empty. """ monkeypatch.setattr(sys, "argv", ["rvl", "index", "create"]) @@ -256,8 +247,8 @@ def test_create_missing_schema(monkeypatch, capsys): assert exc_info.value.code == 2 # Nothing leaks on this error path. assert captured.out == "" - # Exact stderr contract from the missing-schema guard's print() call. - assert captured.err == "Schema must be provided to create an index\n" + # Some explanatory message reaches stderr. + assert captured.err != "" def test_create_schema_input_error(monkeypatch, capsys): @@ -290,9 +281,8 @@ def test_create_schema_input_error(monkeypatch, capsys): def test_create_redis_search_error(monkeypatch, capsys): """Tests that ``rvl index create`` reports Redis-side failures on stderr. - Expected behavior: ``SystemExit`` code is 1, stdout is empty, and stderr - contains both the ``Redis search operation failed for index 'test-idx'.`` - prefix and the underlying error message. + Expected behavior: ``SystemExit`` code is 1, stdout is empty, and the + underlying error message reaches stderr. """ from redisvl.exceptions import RedisSearchError @@ -313,9 +303,7 @@ def test_create_redis_search_error(monkeypatch, capsys): assert exc_info.value.code == 1 # No partial output before the failure. assert captured.out == "" - # Documented error prefix, with the index name surfacing verbatim. - assert "Redis search operation failed for index 'test-idx'." in captured.err - # The exception's str() is forwarded to stderr verbatim. + # The underlying error message reaches stderr so the user knows what failed. assert redis_error_message in captured.err @@ -346,13 +334,21 @@ def test_create_success(monkeypatch, capsys, argv: list[str]): assert captured.err == "" -def test_listall_json(monkeypatch, capsys): +@pytest.mark.parametrize( + "ft_list_result, expected_payload", + [ + ([b"idx_a", b"idx_b"], {"indices": ["idx_a", "idx_b"]}), + ([], {"indices": []}), + ], +) +def test_listall_json(monkeypatch, capsys, ft_list_result, expected_payload): """Tests that ``rvl index listall --json`` prints the documented JSON contract. Expected behavior: no ``SystemExit`` is raised, stdout is one JSON line of the form ``{"indices": [...]}`` (no human banner), and stderr is empty. + Parametrized over a populated and an empty ``FT._LIST`` result. """ - _patch_redis_connection(monkeypatch, result=[b"idx_a", b"idx_b"]) + _patch_redis_connection(monkeypatch, result=ft_list_result) monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) try: @@ -366,7 +362,7 @@ def test_listall_json(monkeypatch, capsys): # Single machine-readable line, nothing else on stdout. assert out.count("\n") == 0 # Stable top-level JSON contract: only the "indices" key, in FT._LIST order. - assert json.loads(out) == {"indices": ["idx_a", "idx_b"]} + assert json.loads(out) == expected_payload # JSON success path is silent on stderr. assert captured.err == "" @@ -388,27 +384,6 @@ def test_listall_table(monkeypatch, capsys): assert captured.err == "" -def test_listall_json_empty(monkeypatch, capsys): - """Tests that ``rvl index listall --json`` handles an empty FT._LIST result. - - Expected behavior: stdout is one JSON line with ``{"indices": []}`` (no - human banner), and stderr is empty. - """ - _patch_redis_connection(monkeypatch) - monkeypatch.setattr(sys, "argv", ["rvl", "index", "listall", "--json"]) - - Index() - captured = capsys.readouterr() - out = captured.out.strip() - - # Single machine-readable line for the empty case too. - assert out.count("\n") == 0 - # Empty array is a valid success payload. - assert json.loads(out) == {"indices": []} - # JSON success path is silent on stderr. - assert captured.err == "" - - @pytest.mark.parametrize( "argv", [ @@ -420,8 +395,8 @@ def test_listall_runtime_error(monkeypatch, capsys, argv: list[str]): """Tests that ``rvl index listall`` reports runtime failures on stderr. Expected behavior: ``SystemExit`` code is 1, stdout is empty (no - half-formed output), and the underlying error message is on stderr. - ``--json`` is parametrized to confirm the contract is uniform. + half-formed output), and stderr is non-empty. ``--json`` is parametrized + to confirm the contract is uniform. """ _patch_redis_connection(monkeypatch, boom=True) monkeypatch.setattr(sys, "argv", argv) @@ -434,15 +409,15 @@ def test_listall_runtime_error(monkeypatch, capsys, argv: list[str]): assert exc_info.value.code == 1 # Failure happens before any rendering - nothing on stdout. assert captured.out == "" - # Underlying error message is surfaced on stderr. - assert "redis unavailable" in captured.err + # The failure is surfaced on stderr. + assert captured.err != "" def test_info_json_normalize(): """Tests that ``_index_info_for_json`` maps FT.INFO lists to structured JSON. - Expected behavior: the input dict is not mutated and the output has the - documented top-level keys ``index_information`` and ``index_fields``. + Expected behavior: the input dict is not mutated and the returned payload + is exactly the documented ``index_information`` + ``index_fields`` shape. """ raw = { "index_name": "test_index", @@ -546,8 +521,8 @@ def test_info_runtime_error(monkeypatch, capsys, argv: list[str]): """Tests that ``rvl index info`` reports runtime failures on stderr. Expected behavior: ``SystemExit`` code is 1, stdout is empty (no - half-formed output), and the underlying error message is on stderr. - ``--json`` is parametrized to confirm the contract is uniform. + half-formed output), and stderr is non-empty. ``--json`` is parametrized + to confirm the contract is uniform. """ _patch_search_index_for_info( monkeypatch, info_behavior=_raise(RuntimeError("boom")) @@ -562,8 +537,8 @@ def test_info_runtime_error(monkeypatch, capsys, argv: list[str]): assert exc_info.value.code == 1 # Failure happens before any rendering - nothing on stdout. assert captured.out == "" - # Underlying error message is surfaced on stderr. - assert "boom" in captured.err + # The failure is surfaced on stderr. + assert captured.err != "" @pytest.mark.parametrize( @@ -574,11 +549,11 @@ def test_info_runtime_error(monkeypatch, capsys, argv: list[str]): ], ) def test_info_no_target(monkeypatch, capsys, argv: list[str]): - """Tests that ``rvl index info`` without ``-i`` or ``-s`` exits with the usage error. + """Tests that ``rvl index info`` without ``-i`` or ``-s`` exits with a usage error. Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr - is exactly ``Index name or schema must be provided\\n``. ``--json`` is - parametrized to confirm no JSON contract is invented for usage errors. + is non-empty. ``--json`` is parametrized to confirm no JSON contract is + invented for usage errors. """ monkeypatch.setattr(sys, "argv", argv) @@ -588,18 +563,18 @@ def test_info_no_target(monkeypatch, capsys, argv: list[str]): # _connect_to_index uses argparse-style usage exit code assert exc_info.value.code == 2 - # usage errors must not pollute stdout, even with --json + # Usage errors must not pollute stdout, even with --json. assert captured.out == "" - # exact stderr message contract from _connect_to_index - assert captured.err == "Index name or schema must be provided\n" + # Some explanatory message reaches stderr; exact wording is not part of the contract. + assert captured.err != "" def test_info_table(monkeypatch, capsys): - """Tests that default ``rvl index info`` prints the human-readable table. + """Tests that default ``rvl index info`` runs to completion on the table path. - Expected behavior: stdout includes the ``Index Information:`` and - ``Index Fields:`` banners and surfaces the index name and storage type; - stderr is empty. + Expected behavior: no ``SystemExit`` is raised, stdout is non-empty, and + stderr is empty. Exact rendering is a tabulate-library detail; data + correctness is pinned by ``test_info_json``. """ _patch_search_index_for_info(monkeypatch) monkeypatch.setattr(sys, "argv", ["rvl", "index", "info", "-i", "test-idx"]) @@ -607,16 +582,10 @@ def test_info_table(monkeypatch, capsys): Index() captured = capsys.readouterr() + # Table path produces some output - the renderer ran and printed cells. + assert captured.out != "" # Table success is silent on stderr. assert captured.err == "" - # Top-of-table banner is printed. - assert "Index Information:" in captured.out - # Fields-table banner is printed. - assert "Index Fields:" in captured.out - # The index name surfaces in the rendered cells. - assert "test-idx" in captured.out - # The storage type surfaces in the rendered cells. - assert "HASH" in captured.out @pytest.mark.parametrize( @@ -627,11 +596,10 @@ def test_info_table(monkeypatch, capsys): ], ) def test_info_missing_index(monkeypatch, capsys, argv: list[str]): - """Tests that ``rvl index info -i `` reports the missing index on stderr. + """Tests that ``rvl index info -i `` reports the failure on stderr. - Expected behavior: ``SystemExit`` code is 1, stdout is empty, and stderr - contains both the ``Redis search operation failed for index 'test-idx'.`` - prefix and the underlying error message. ``--json`` is parametrized to + Expected behavior: ``SystemExit`` code is 1, stdout is empty, and the + underlying error message reaches stderr. ``--json`` is parametrized to confirm the contract does not change. """ from redisvl.exceptions import RedisSearchError @@ -651,7 +619,3 @@ def test_info_missing_index(monkeypatch, capsys, argv: list[str]): assert exc_info.value.code == 1 # Nothing leaks on this error path. assert captured.out == "" - # Documented error prefix, with the index name surfacing verbatim. - assert "Redis search operation failed for index 'test-idx'." in captured.err - # The exception's str() is forwarded to stderr verbatim. - assert underlying_error in captured.err diff --git a/tests/unit/test_cli_main.py b/tests/unit/test_cli_main.py index d87c9caa..8359ab43 100644 --- a/tests/unit/test_cli_main.py +++ b/tests/unit/test_cli_main.py @@ -10,16 +10,10 @@ def _assert_help_contract(help_text: str) -> None: - """Assert key help text users rely on: description, usage, and commands.""" - # Includes the CLI description. - assert "Redis Vector Library CLI" in help_text - # Includes the command section header. - assert "Command groups:" in help_text + """Assert that ``rvl`` help lists every supported top-level command.""" for name in _COMMANDS: - # Includes each supported top-level command. + # Each supported top-level command appears on its own help line. assert re.search(rf"^\s*{re.escape(name)}\s+", help_text, re.MULTILINE) - # Includes the short usage form. - assert "rvl []" in help_text @pytest.mark.parametrize("argv", [["rvl"], ["rvl", "--help"], ["rvl", "-h"]]) @@ -60,8 +54,6 @@ def test_unknown_command(monkeypatch, capsys): for name in _COMMANDS: # stderr help still lists every valid top-level command. assert name in out.err - # stderr includes the command section header from help. - assert "Command groups:" in out.err def test_subprocess_module_help(): From bd2de14faea6fa269d77a8f5990e8bb9fae7cf2b Mon Sep 17 00:00:00 2001 From: limjoobin Date: Wed, 6 May 2026 17:26:19 +0800 Subject: [PATCH 36/36] test: Aded tests for stats command in cli --- tests/unit/test_cli_stats.py | 314 +++++++++++++++++++++++++++-------- 1 file changed, 249 insertions(+), 65 deletions(-) diff --git a/tests/unit/test_cli_stats.py b/tests/unit/test_cli_stats.py index ec769706..4d5a42a4 100644 --- a/tests/unit/test_cli_stats.py +++ b/tests/unit/test_cli_stats.py @@ -1,4 +1,5 @@ import json +import subprocess import sys import pytest @@ -6,105 +7,288 @@ from redisvl.cli.stats import STATS_KEYS, Stats, _stats_rows -def test_stats_rows_includes_all_stable_top_level_keys_in_order(): - """``_stats_rows({})`` returns the full ordered row list for an empty index info. +@pytest.mark.parametrize("argv", [["rvl", "stats", "--help"], ["rvl", "stats", "-h"]]) +def test_rvl_stats_help(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl stats --help`` and ``-h`` are discoverable. - Expected behavior: produces a complete, ``STATS_KEYS``-ordered set of rows - regardless of input; preserves value types at this layer; and represents - missing keys as ``None`` so JSON output remains machine-readable. + Expected behavior: ``SystemExit`` code is 0, stdout is non-empty, and + stderr is empty. """ - data = dict(_stats_rows({})) - assert list(data.keys()) == list(STATS_KEYS) # column order matches STATS_KEYS - assert all(data[k] is None for k in STATS_KEYS) # missing index_info keys -> None + monkeypatch.setattr(sys, "argv", argv) + with pytest.raises(SystemExit) as exc_info: + Stats() + captured = capsys.readouterr() -def test_stats_json_prints_only_json_to_stdout(monkeypatch, capsys): - """``rvl stats -i --json`` writes only a JSON object to stdout. + # Help requests terminate successfully. + assert exc_info.value.code == 0 + # Help is rendered to stdout, not stderr - critical for shell redirection. + assert captured.out != "" + # Successful help output does not leak to stderr. + assert captured.err == "" - Uses a fake ``SearchIndex`` so no Redis is required. - Expected behavior: ``--json`` skips ``_display_stats`` and emits one - single-line JSON document with the full ``STATS_KEYS`` schema and - native values (e.g. ``num_docs=7`` -> ``7``). +def test_rvl_stats_subprocess_help(): + """End-to-end smoke test of ``rvl stats --help`` via the runner module. - Row order is covered by ``test_stats_rows_*``; JSON key order is covered - by ``test_cli_print_json_preserves_key_order``. + Expected behavior: the subprocess exits 0, stdout is non-empty, and + stderr is empty. """ + result = subprocess.run( + [sys.executable, "-m", "redisvl.cli.runner", "stats", "--help"], + check=False, + capture_output=True, + text=True, + ) + # Process exited cleanly. + assert result.returncode == 0 + # Help is rendered to stdout, not stderr. + assert result.stdout != "" + # Help is not emitted on stderr. + assert result.stderr == "" - class FakeIndex: - def __init__(self, *a, **k): - pass - def info(self): - return {"num_docs": 7} +def _raise(exc: BaseException): + """Return a zero-arg callable that raises ``exc``.""" - monkeypatch.setattr("redisvl.cli.stats.SearchIndex", FakeIndex) - monkeypatch.setattr(sys, "argv", ["rvl", "stats", "-i", "test-idx", "--json"]) - Stats() - out = capsys.readouterr().out.strip() - assert "Statistics" not in out # --json must not emit the table UI text - assert out.count("\n") == 0 # exactly one JSON object on stdout, no extra lines - payload = json.loads(out) - assert set(payload) == set(STATS_KEYS) # same stat keys as the shared schema list - assert payload["num_docs"] == 7 # numbers remain numbers for machine consumers - assert payload["num_terms"] is None # missing values become JSON null, not "None" + def _do(): + raise exc + return _do -def test_stats_default_prints_table(monkeypatch, capsys): - """``rvl stats -i `` without ``--json`` still renders the ASCII table. - Expected behavior: ``Stats.stats`` selects the human-readable branch and - delegates to ``_display_stats``; the ``Statistics:`` banner is the signal - that the table path ran. Guards against the ``--json`` plumbing regressing - the default mode. +def _patch_search_index_for_stats( + monkeypatch, + *, + info_behavior=None, + index_name: str = "test-idx", +) -> None: + """Patch ``redisvl.cli.stats.SearchIndex`` with a minimal fake for ``rvl stats`` tests. + + By default builds a happy-path FakeIndex whose ``info()`` returns + ``{"num_docs": 7}`` - one populated stat key, the rest absent so the + missing-key -> ``None`` path is exercised. Pass + ``info_behavior=_raise(SomeError())`` to make ``info()`` raise instead. + ``index_name`` is surfaced via ``index.schema.index.name`` so + ``exit_redis_search_error`` can format its message verbatim. """ + if info_behavior is None: + + def info_behavior(): + return {"num_docs": 7} + + class _FakeIndexInfo: + name = index_name + + class _FakeSchema: + index = _FakeIndexInfo() class FakeIndex: + schema = _FakeSchema() + def __init__(self, *a, **k): + # Absorb the ``schema=`` and ``redis_url=`` kwargs that + # ``_connect_to_index`` passes to ``SearchIndex(...)`` directly. + # Without this, Python's default ``__init__`` rejects them. pass + @classmethod + def from_yaml(cls, *_args, **_kwargs): + return cls() + def info(self): - return {"num_docs": 1} + return info_behavior() monkeypatch.setattr("redisvl.cli.stats.SearchIndex", FakeIndex) - monkeypatch.setattr(sys, "argv", ["rvl", "stats", "-i", "test-idx"]) - Stats() - out = capsys.readouterr().out - assert "Statistics:" in out # non-JSON path prints the table header line -def test_stats_missing_index_and_schema_exits_zero_without_json(monkeypatch, capsys): - """Without -i/-s, ``_connect_to_index`` logs and ``exit(0)`` s; no JSON leaks. +def _patch_search_index_from_yaml_raises(monkeypatch, exc: BaseException) -> None: + """Patch ``redisvl.cli.stats.SearchIndex.from_yaml`` to raise ``exc``. + + Used by the schema-input-error path where ``-s `` is provided but + loading fails (e.g. file missing, malformed YAML). + """ + + class FakeSearchIndex: + @classmethod + def from_yaml(cls, *_args, **_kwargs): + raise exc + + monkeypatch.setattr("redisvl.cli.stats.SearchIndex", FakeSearchIndex) + - Expected behavior: invalid input follows the standard ``rvl`` "log + exit 0" - pattern. ``--json`` does not relax that contract — stdout stays empty so - machine consumers never see a half-formed JSON object. +@pytest.mark.parametrize( + "argv", + [ + ["rvl", "stats"], + ["rvl", "stats", "--json"], + ], +) +def test_stats_no_target(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl stats`` without ``-i`` or ``-s`` exits with a usage error. + + Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr + is non-empty. ``--json`` is parametrized to confirm no JSON contract is + invented for usage errors. """ - monkeypatch.setattr(sys, "argv", ["rvl", "stats", "--json"]) - with pytest.raises(SystemExit) as excinfo: + monkeypatch.setattr(sys, "argv", argv) + + with pytest.raises(SystemExit) as exc_info: Stats() - assert excinfo.value.code == 2 - assert capsys.readouterr().out == "" # no JSON object emitted on error + captured = capsys.readouterr() + # _connect_to_index uses argparse-style usage-error exit code. + assert exc_info.value.code == 2 + # Usage errors must not pollute stdout, even with --json. + assert captured.out == "" + # Some explanatory message reaches stderr; exact wording is not part of the contract. + assert captured.err != "" -def test_stats_info_failure_exits_zero_without_json(monkeypatch, capsys): - """If ``index.info()`` raises, ``Stats.__init__`` logs and ``exit(0)`` s; no JSON leaks. - Expected behavior: ``try/except Exception`` converts backend failures into - ``exit(0)`` (no traceback). With ``--json``, stdout stays empty so "exit 0 - + empty stdout" means "no result", never "malformed result". +def test_stats_schema_input_error(monkeypatch, capsys): + """Tests that ``rvl stats -s `` reports schema-load failures on stderr. + + Expected behavior: ``SystemExit`` code is 2, stdout is empty, and stderr + is non-empty. """ + _patch_search_index_from_yaml_raises( + monkeypatch, FileNotFoundError("schema file missing: /does/not/exist.yaml") + ) + monkeypatch.setattr(sys, "argv", ["rvl", "stats", "-s", "/does/not/exist.yaml"]) - class BoomIndex: - def __init__(self, *a, **k): - pass + with pytest.raises(SystemExit) as exc_info: + Stats() + captured = capsys.readouterr() + + # exit_schema_input_error uses exit code 2 when -s was provided. + assert exc_info.value.code == 2 + # Nothing on stdout when schema input fails. + assert captured.out == "" + # The failure is surfaced on stderr. + assert captured.err != "" - def info(self): - raise RuntimeError("boom") - monkeypatch.setattr("redisvl.cli.stats.SearchIndex", BoomIndex) +@pytest.mark.parametrize( + "argv", + [ + ["rvl", "stats", "-i", "test-idx"], + ["rvl", "stats", "-i", "test-idx", "--json"], + ], +) +def test_stats_redis_search_error(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl stats`` reports Redis-side failures on stderr. + + Expected behavior: ``SystemExit`` code is 1, stdout is empty, and stderr + is non-empty. ``--json`` is parametrized to confirm the contract is + uniform. + """ + from redisvl.exceptions import RedisSearchError + + _patch_search_index_for_stats( + monkeypatch, info_behavior=_raise(RedisSearchError("Unknown index name")) + ) + monkeypatch.setattr(sys, "argv", argv) + + with pytest.raises(SystemExit) as exc_info: + Stats() + captured = capsys.readouterr() + + # Documented Redis-failure exit code from exit_redis_search_error. + assert exc_info.value.code == 1 + # No partial output before the failure. + assert captured.out == "" + # The failure is surfaced on stderr. + assert captured.err != "" + + +@pytest.mark.parametrize( + "argv", + [ + ["rvl", "stats", "-i", "test-idx"], + ["rvl", "stats", "-i", "test-idx", "--json"], + ], +) +def test_stats_runtime_error(monkeypatch, capsys, argv: list[str]): + """Tests that ``rvl stats`` reports runtime failures on stderr. + + Expected behavior: ``SystemExit`` code is 1, stdout is empty (no + half-formed output), and stderr is non-empty. ``--json`` is parametrized + to confirm the contract is uniform. + """ + _patch_search_index_for_stats( + monkeypatch, info_behavior=_raise(RuntimeError("boom")) + ) + monkeypatch.setattr(sys, "argv", argv) + + with pytest.raises(SystemExit) as exc_info: + Stats() + captured = capsys.readouterr() + + # Generic runtime failure exits 1 from __init__'s catch-all. + assert exc_info.value.code == 1 + # Failure happens before any rendering - nothing on stdout. + assert captured.out == "" + # The failure is surfaced on stderr; exact wording is not part of the contract. + assert captured.err != "" + + +def test_stats_json(monkeypatch, capsys): + """Tests that ``rvl stats --json`` prints the documented JSON contract. + + Expected behavior: no ``SystemExit`` is raised, stdout is one parseable + JSON line whose keys are exactly ``STATS_KEYS`` in order, native value + types are preserved, and missing input keys become JSON ``null``. + Stderr is empty. + """ + _patch_search_index_for_stats(monkeypatch) monkeypatch.setattr(sys, "argv", ["rvl", "stats", "-i", "test-idx", "--json"]) - with pytest.raises(SystemExit) as excinfo: + + try: Stats() - assert excinfo.value.code == 1 - assert capsys.readouterr().out == "" + except SystemExit as exc: + # Success path must return cleanly, not call sys.exit. + pytest.fail(f"stats --json raised SystemExit({exc.code}) on the success path") + captured = capsys.readouterr() + out = captured.out.strip() + + # Single machine-readable line, nothing else on stdout. + assert out.count("\n") == 0 + payload = json.loads(out) + # Stable top-level JSON contract: every STATS_KEY in order, no extras. + assert list(payload) == list(STATS_KEYS) + # Native types are preserved for machine consumers. + assert payload["num_docs"] == 7 + # Missing input keys deserialize to None and is not mistyped. + assert payload["num_terms"] is None + # JSON success path is silent on stderr. + assert captured.err == "" + + +def test_stats_table(monkeypatch, capsys): + """Tests that default ``rvl stats`` runs the human-readable path to completion. + + Expected behavior: no ``SystemExit`` is raised, stdout is non-empty + (the table renderer ran), and stderr is empty. + """ + _patch_search_index_for_stats(monkeypatch) + monkeypatch.setattr(sys, "argv", ["rvl", "stats", "-i", "test-idx"]) + Stats() + captured = capsys.readouterr() + assert captured.out != "" + assert captured.err == "" + + +def test_stats_rows_shape(): + """Tests that ``_stats_rows`` produces the documented row contract. + + Expected behavior: for an empty ``index_info`` dict, the helper returns + ordered ``(key, None)`` pairs whose keys are exactly ``STATS_KEYS`` in + order - the contract the JSON output relies on. + """ + rows = _stats_rows({}) + data = dict(rows) + + # Every STATS_KEY appears, in declared order. + assert list(data.keys()) == list(STATS_KEYS) + # Missing input keys become None at this layer (serialized as JSON null). + assert all(data[k] is None for k in STATS_KEYS)