Merge pull request #37 from remyxai/terry/fix-pre-existing-test-failures

Fix 6 pre-existing test failures (REMYX-75)

Author: smellslikeml

remyxai/cli/commands.py

@@ -42,7 +42,7 @@ def list_models():
     try:
         handle_model_action({"subaction": "list"})
     except Exception as e:
-        click.echo(f"Error listing models: {e}")
+        raise click.ClickException(f"listing models failed: {e}")
 
 
 @cli.command()
@@ -52,7 +52,7 @@ def summarize_model(model_name):
     try:
         handle_model_action({"subaction": "summarize", "model_name": model_name})
     except Exception as e:
-        click.echo(f"Error summarizing model: {e}")
+        raise click.ClickException(f"summarizing model failed: {e}")
 
 
 @cli.command()
@@ -63,7 +63,7 @@ def deploy_model(model_name, action):
     try:
         handle_deployment_action({"model_name": model_name, "action": action})
     except Exception as e:
-        click.echo(f"Error deploying model: {e}")
+        raise click.ClickException(f"deploying model failed: {e}")
 
 
 @cli.command()
@@ -74,7 +74,7 @@ def dataset(action, dataset_name=None):
     try:
         handle_dataset_action({"action": action, "dataset_name": dataset_name})
     except Exception as e:
-        click.echo(f"Error managing dataset: {e}")
+        raise click.ClickException(f"dataset action failed: {e}")
 
 
 @cli.group()

tests/api/test_dataset.py

@@ -1,30 +1,45 @@
-from remyxai.api.datasets import list_datasets, delete_dataset, download_dataset
 from unittest.mock import patch
-from remyxai.api.datasets import BASE_URL
+
+from remyxai.api.datasets import BASE_URL, delete_dataset, download_dataset, list_datasets
+
 
 @patch("remyxai.api.datasets.requests.get")
 def test_list_datasets(mock_get):
+    """list_datasets reads from the response's 'message' field."""
     mock_get.return_value.status_code = 200
-    mock_get.return_value.json.return_value = {"datasets": ["dataset1", "dataset2"]}
+    mock_get.return_value.json.return_value = {"message": ["dataset1", "dataset2"]}
+
     datasets = list_datasets()
+
     assert isinstance(datasets, list)
-    assert len(datasets) > 0
+    assert datasets == ["dataset1", "dataset2"]
+
 
 @patch("remyxai.api.datasets.requests.delete")
 def test_delete_dataset(mock_delete):
+    """delete_dataset requires (dataset_type, dataset_name)."""
     mock_delete.return_value.status_code = 200
     mock_delete.return_value.json.return_value = {"message": "Dataset deleted successfully"}
-    dataset_name = "test_dataset"
-    delete_dataset(dataset_name)
-    assert mock_delete.called_once_with(f"{BASE_URL}/datasets/delete/{dataset_name}")
+
+    result = delete_dataset("eval", "test_dataset")
+
+    assert result == "Dataset deleted successfully"
+    args, _ = mock_delete.call_args
+    assert args[0] == f"{BASE_URL}/datasets/delete/eval/test_dataset"
+
 
 @patch("remyxai.api.datasets.requests.get")
 def test_download_dataset(mock_get):
+    """download_dataset requires (dataset_type, dataset_name); returns an error
+    when no presigned_url is in the response (avoids touching the network)."""
     mock_get.return_value.status_code = 200
-    mock_get.return_value.json.return_value = {"url": "https://example.com/dataset.zip"}
-    dataset_name = "test_dataset"
-    download_dataset(dataset_name)
-    assert mock_get.called_once_with(f"{BASE_URL}/datasets/download/{dataset_name}")
-
+    mock_get.return_value.json.return_value = {"presigned_url": ""}
 
+    result = download_dataset("eval", "test_dataset")
 
+    # With an empty presigned URL the function returns an explicit error dict
+    # rather than attempting a download. Keep the test hermetic.
+    assert isinstance(result, dict)
+    assert "error" in result
+    args, _ = mock_get.call_args
+    assert args[0] == f"{BASE_URL}/datasets/download/eval/test_dataset"

tests/cli/test_commands.py

@@ -54,8 +54,13 @@ def test_deploy_model_down(mock_handle_deployment_action):
 
 
 def test_deploy_model_invalid_action():
+    """Invalid action must surface as an error with a non-zero exit code.
+
+    The command uses `click.ClickException`, which prefixes the message
+    with `Error: ` and exits 1. This is the contract this test enforces.
+    """
     runner = CliRunner()
     result = runner.invoke(cli, ["deploy-model", "model_name", "invalid_action"])
 
-    assert "Error deploying model" in result.output
+    assert "deploying model failed" in result.output
     assert result.exit_code != 0

tests/conftest.py

@@ -0,0 +1,11 @@
+"""Shared pytest setup.
+
+Sets `REMYXAI_API_KEY` before any test module imports `remyxai.api`,
+so the module-level `HEADERS` dict (built at import time in
+`remyxai/api/__init__.py`) has a non-empty Bearer token. Per-test
+autouse fixtures elsewhere can't fix this on their own because they
+run after collection-time imports.
+"""
+import os
+
+os.environ.setdefault("REMYXAI_API_KEY", "test-key-collection-time")

tests/integration/test_recommendations_live.py

@@ -20,10 +20,18 @@
 import pytest
 
 # ─── skip entire module if no API key ────────────────────────────────────────
-
+#
+# Skips when the env var is missing OR is a unit-test placeholder. The
+# tests/conftest.py sets `REMYXAI_API_KEY=test-...` at collection time so
+# the module-level HEADERS in remyxai/api/__init__.py is built with a
+# non-empty Bearer token (required for the unit-test header-threading
+# assertions). Live integration tests against engine.remyx.ai only run
+# when the env var is a real, non-test key.
+
+_API_KEY = os.environ.get("REMYXAI_API_KEY", "")
 pytestmark = pytest.mark.skipif(
-    not os.environ.get("REMYXAI_API_KEY"),
-    reason="REMYXAI_API_KEY not set — skipping live integration tests",
+    not _API_KEY or _API_KEY.startswith("test-"),
+    reason="REMYXAI_API_KEY not set (or is a unit-test placeholder) — skipping live integration tests",
 )