Honor bool/Optional return contracts from weaviate-python-client

Several methods of the weaviate-python-client are not the usual
"raise on failure" style. Instead they accept multiple HTTP status
codes as "ok" and return:

  * bool        -- False means a logical no-op (404 not found,
                   409 already in target state, ...)
  * Optional[X] -- None means "not found" (404)

The CLI was discarding those returns and silently reporting success
on no-ops -- e.g. `delete user --user_name=ghost` printed
"User 'ghost' deleted successfully." with exit code 0.

Surface the no-op as a clear error in the affected managers:

  * UserManager
      - delete_user      : raise on False (404)
      - update_user      : raise on False from activate()/deactivate()
                           (409 already in target state)
      - get_user         : raise on None (404) when looking up by name
  * AliasManager
      - delete_alias     : raise on False (404)
      - update_alias     : raise on False (404)
      - get_alias        : fix return type hint to Optional[AliasReturn]
  * DataManager
      - __delete_data    : raise on False from data.delete_by_id()
                           (404) in the uuid path

The CLI commands already wrap manager calls in `try/except Exception`
and exit 1 with `Error: ...`, so no command-layer changes were needed
beyond simplifying delete_user_cli (already on this branch).

Add regression coverage on two layers:

  * Manager unit tests for each new bool=False / None path
    (test_user_manager.py, test_alias_manager.py, test_data_manager.py)
  * End-to-end CliRunner tests in
    test/unittests/test_cli_bool_contract.py that pin the contract:
    a no-op exits non-zero with a clear message and never prints a
    success line. If a future contributor swallows a bool again,
    these tests fail loudly.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: jfrancoa

test/unittests/test_cli_bool_contract.py

@@ -0,0 +1,289 @@
+"""
+End-to-end CLI regression tests for the bool / Optional contract.
+
+Several methods of the weaviate-python-client are *not* the usual "raise on
+failure" style. Instead, they accept multiple HTTP status codes as "ok" and
+return:
+
+  * ``bool``         -- ``False`` means a logical no-op (404 not found, 409
+                        already in target state, ...)
+  * ``Optional[X]``  -- ``None`` means "not found" (404)
+
+The CLI must surface those as a non-zero exit code with a clear ``Error:``
+message -- never silently report success. These tests lock in that contract
+end-to-end (CliRunner -> command -> manager -> mocked client method) so the
+bug class cannot creep back in.
+
+If you add a new caller of one of these client APIs, add a test here.
+"""
+
+import pytest
+from click.testing import CliRunner
+from unittest.mock import MagicMock, patch
+
+from cli import main
+
+
+@pytest.fixture
+def cli_runner() -> CliRunner:
+    return CliRunner()
+
+
+@pytest.fixture
+def fake_client() -> MagicMock:
+    """A MagicMock standing in for a real WeaviateClient."""
+    return MagicMock()
+
+
+def _invoke(cli_runner: CliRunner, fake_client: MagicMock, command_module: str, args):
+    """Invoke ``main`` with ``args`` while patching
+    ``get_client_from_context`` in the targeted command module so the CLI gets
+    our fake client instead of opening a real connection."""
+    target = f"{command_module}.get_client_from_context"
+    with patch(target, return_value=fake_client):
+        return cli_runner.invoke(main, args)
+
+
+# ---------------------------------------------------------------------------
+# delete user -- client.users.db.delete() returns False on 404
+# ---------------------------------------------------------------------------
+
+
+def test_cli_delete_user_not_found_exits_nonzero(cli_runner, fake_client):
+    fake_client.users.db.delete.return_value = False
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.delete",
+        ["delete", "user", "--user_name", "ghost"],
+    )
+
+    assert result.exit_code == 1, result.output
+    assert "User 'ghost' not found." in result.output
+    assert "deleted successfully" not in result.output
+
+
+def test_cli_delete_user_success(cli_runner, fake_client):
+    fake_client.users.db.delete.return_value = True
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.delete",
+        ["delete", "user", "--user_name", "alice"],
+    )
+
+    assert result.exit_code == 0, result.output
+    assert "alice" in result.output
+    assert "deleted successfully" in result.output
+
+
+# ---------------------------------------------------------------------------
+# update user --activate / --deactivate -- bool=False on 409
+# ---------------------------------------------------------------------------
+
+
+def test_cli_update_user_activate_already_active_exits_nonzero(cli_runner, fake_client):
+    fake_client.users.db.activate.return_value = False
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.update",
+        ["update", "user", "--user_name", "alice", "--activate"],
+    )
+
+    assert result.exit_code == 1, result.output
+    assert "already active" in result.output
+    assert "activated successfully" not in result.output
+
+
+def test_cli_update_user_activate_success(cli_runner, fake_client):
+    fake_client.users.db.activate.return_value = True
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.update",
+        ["update", "user", "--user_name", "alice", "--activate"],
+    )
+
+    assert result.exit_code == 0, result.output
+    assert "activated successfully" in result.output
+
+
+def test_cli_update_user_deactivate_already_deactivated_exits_nonzero(
+    cli_runner, fake_client
+):
+    fake_client.users.db.deactivate.return_value = False
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.update",
+        ["update", "user", "--user_name", "alice", "--deactivate"],
+    )
+
+    assert result.exit_code == 1, result.output
+    assert "already deactivated" in result.output
+    assert "deactivated successfully" not in result.output
+
+
+def test_cli_update_user_deactivate_success(cli_runner, fake_client):
+    fake_client.users.db.deactivate.return_value = True
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.update",
+        ["update", "user", "--user_name", "alice", "--deactivate"],
+    )
+
+    assert result.exit_code == 0, result.output
+    assert "deactivated successfully" in result.output
+
+
+# ---------------------------------------------------------------------------
+# get user -- client.users.db.get() returns None on 404
+# ---------------------------------------------------------------------------
+
+
+def test_cli_get_user_not_found_exits_nonzero(cli_runner, fake_client):
+    fake_client.users.db.get.return_value = None
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.get",
+        ["get", "user", "--user_name", "ghost"],
+    )
+
+    assert result.exit_code == 1, result.output
+    assert "User 'ghost' not found." in result.output
+
+
+# ---------------------------------------------------------------------------
+# delete alias -- client.alias.delete() returns False on 404
+# ---------------------------------------------------------------------------
+
+
+def test_cli_delete_alias_not_found_exits_nonzero(cli_runner, fake_client):
+    fake_client.alias.delete.return_value = False
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.delete",
+        ["delete", "alias", "ghost_alias"],
+    )
+
+    assert result.exit_code == 1, result.output
+    assert "Alias 'ghost_alias' not found." in result.output
+    assert "deleted successfully" not in result.output
+
+
+def test_cli_delete_alias_success(cli_runner, fake_client):
+    fake_client.alias.delete.return_value = True
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.delete",
+        ["delete", "alias", "my_alias"],
+    )
+
+    assert result.exit_code == 0, result.output
+    assert "deleted successfully" in result.output
+
+
+# ---------------------------------------------------------------------------
+# update alias -- client.alias.update() returns False on 404
+# ---------------------------------------------------------------------------
+
+
+def test_cli_update_alias_not_found_exits_nonzero(cli_runner, fake_client):
+    fake_client.alias.update.return_value = False
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.update",
+        ["update", "alias", "ghost", "MyCollection"],
+    )
+
+    assert result.exit_code == 1, result.output
+    assert "Alias 'ghost' not found." in result.output
+    assert "updated successfully" not in result.output
+
+
+def test_cli_update_alias_success(cli_runner, fake_client):
+    fake_client.alias.update.return_value = True
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.update",
+        ["update", "alias", "my_alias", "MyCollection"],
+    )
+
+    assert result.exit_code == 0, result.output
+    assert "updated successfully" in result.output
+
+
+# ---------------------------------------------------------------------------
+# delete data --uuid -- collection.data.delete_by_id() returns False on 404
+# ---------------------------------------------------------------------------
+
+
+def test_cli_delete_data_by_uuid_not_found_exits_nonzero(cli_runner, fake_client):
+    # Pretend the collection exists and is not multi-tenant.
+    fake_client.collections.exists.return_value = True
+    mock_collection = MagicMock()
+    mock_collection.name = "TestCollection"
+    mock_collection.config.get.return_value.multi_tenancy_config.enabled = False
+    mock_collection.with_consistency_level.return_value.data.delete_by_id.return_value = (
+        False
+    )
+    fake_client.collections.get.return_value = mock_collection
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.delete",
+        [
+            "delete",
+            "data",
+            "--collection",
+            "TestCollection",
+            "--uuid",
+            "00000000-0000-0000-0000-000000000000",
+        ],
+    )
+
+    assert result.exit_code == 1, result.output
+    assert "00000000-0000-0000-0000-000000000000" in result.output
+    assert "not found" in result.output
+
+
+# ---------------------------------------------------------------------------
+# get alias -- client.alias.get() returns None on 404 (CLI handles it
+# explicitly, but pin the contract).
+# ---------------------------------------------------------------------------
+
+
+def test_cli_get_alias_not_found_reports_not_found(cli_runner, fake_client):
+    fake_client.alias.get.return_value = None
+
+    result = _invoke(
+        cli_runner,
+        fake_client,
+        "weaviate_cli.commands.get",
+        ["get", "alias", "--alias_name", "ghost"],
+    )
+
+    # The get command currently treats a missing alias as a soft miss (exit 0
+    # with an explanatory line). The important guard is that we never silently
+    # render a None alias as if it existed.
+    assert result.exit_code == 0, result.output
+    assert "Alias 'ghost' not found." in result.output

test/unittests/test_managers/test_alias_manager.py

@@ -58,6 +58,7 @@ def test_update_alias_success(
     """
     alias_name = "test_alias"
     collection_name = "NewTestCollection"
+    mock_client.alias.update.return_value = True
 
     alias_manager.update_alias(alias_name, collection_name)
 
@@ -66,6 +67,22 @@ def test_update_alias_success(
     )
 
 
+def test_update_alias_not_found_raises(
+    alias_manager: AliasManager, mock_client: MagicMock
+) -> None:
+    """The client returns False on 404 instead of raising."""
+    alias_name = "missing_alias"
+    mock_client.alias.update.return_value = False
+
+    with pytest.raises(Exception) as exc_info:
+        alias_manager.update_alias(alias_name, "TargetCollection")
+
+    assert (
+        f"Error updating alias '{alias_name}': Alias '{alias_name}' not found."
+        in str(exc_info.value)
+    )
+
+
 def test_update_alias_error(
     alias_manager: AliasManager, mock_client: MagicMock
 ) -> None:
@@ -120,12 +137,29 @@ def test_delete_alias_success(
     Test successful alias deletion.
     """
     alias_name = "test_alias"
+    mock_client.alias.delete.return_value = True
 
     alias_manager.delete_alias(alias_name)
 
     mock_client.alias.delete.assert_called_once_with(alias_name=alias_name)
 
 
+def test_delete_alias_not_found_raises(
+    alias_manager: AliasManager, mock_client: MagicMock
+) -> None:
+    """The client returns False on 404 instead of raising."""
+    alias_name = "missing_alias"
+    mock_client.alias.delete.return_value = False
+
+    with pytest.raises(Exception) as exc_info:
+        alias_manager.delete_alias(alias_name)
+
+    assert (
+        f"Error deleting alias '{alias_name}': Alias '{alias_name}' not found."
+        in str(exc_info.value)
+    )
+
+
 def test_delete_alias_error(
     alias_manager: AliasManager, mock_client: MagicMock
 ) -> None:
@@ -219,6 +253,7 @@ def test_delete_alias_success_text(
     alias_manager: AliasManager, mock_client: MagicMock, capsys
 ) -> None:
     """Test delete_alias emits a text success message."""
+    mock_client.alias.delete.return_value = True
     alias_manager.delete_alias("my_alias", json_output=False)
 
     mock_client.alias.delete.assert_called_once_with(alias_name="my_alias")
@@ -231,6 +266,7 @@ def test_delete_alias_success_json(
     alias_manager: AliasManager, mock_client: MagicMock, capsys
 ) -> None:
     """Test delete_alias emits a JSON success message."""
+    mock_client.alias.delete.return_value = True
     alias_manager.delete_alias("my_alias", json_output=True)
 
     out = capsys.readouterr().out
@@ -283,6 +319,7 @@ def test_update_alias_json_output(
     alias_manager: AliasManager, mock_client: MagicMock, capsys
 ) -> None:
     """Test update_alias emits JSON when json_output=True."""
+    mock_client.alias.update.return_value = True
     alias_manager.update_alias("my_alias", "NewCollection", json_output=True)
 
     out = capsys.readouterr().out
@@ -295,6 +332,7 @@ def test_update_alias_text_output(
     alias_manager: AliasManager, mock_client: MagicMock, capsys
 ) -> None:
     """Test update_alias emits text when json_output=False."""
+    mock_client.alias.update.return_value = True
     alias_manager.update_alias("my_alias", "NewCollection", json_output=False)
 
     out = capsys.readouterr().out