From 2559ecddbaab22c6e1aef31c642ef59044d0f74a Mon Sep 17 00:00:00 2001 From: nupursharma-labellerr Date: Mon, 22 Dec 2025 22:17:18 +0530 Subject: [PATCH 01/11] [LABIMP-8413] Adding validation for invalid dataset id --- labellerr/core/datasets/base.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/labellerr/core/datasets/base.py b/labellerr/core/datasets/base.py index 2dda8a9..47aa550 100644 --- a/labellerr/core/datasets/base.py +++ b/labellerr/core/datasets/base.py @@ -29,6 +29,12 @@ def _register(cls, data_type, dataset_class): @staticmethod def get_dataset(client: "LabellerrClient", dataset_id: str): """Get dataset from Labellerr API""" + # Validate dataset_id format (should be a valid UUID) + try: + uuid.UUID(dataset_id) + except (ValueError, AttributeError): + raise InvalidDatasetError(f"Invalid dataset ID format: {dataset_id}") + unique_id = str(uuid.uuid4()) url = ( f"{constants.BASE_URL}/datasets/{dataset_id}?client_id={client.client_id}" From a21e154f387bd890382229dfc293c2f72666f1a1 Mon Sep 17 00:00:00 2001 From: nupursharma-labellerr Date: Mon, 22 Dec 2025 22:18:25 +0530 Subject: [PATCH 02/11] [LABIMP-8413] Adding validation for invalid dataset id --- labellerr/core/datasets/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/labellerr/core/datasets/base.py b/labellerr/core/datasets/base.py index 47aa550..acade55 100644 --- a/labellerr/core/datasets/base.py +++ b/labellerr/core/datasets/base.py @@ -29,12 +29,12 @@ def _register(cls, data_type, dataset_class): @staticmethod def get_dataset(client: "LabellerrClient", dataset_id: str): """Get dataset from Labellerr API""" - # Validate dataset_id format (should be a valid UUID) + # Validate dataset_id format (should be a valid UUID) try: uuid.UUID(dataset_id) except (ValueError, AttributeError): raise InvalidDatasetError(f"Invalid dataset ID format: {dataset_id}") - + unique_id = str(uuid.uuid4()) url = ( f"{constants.BASE_URL}/datasets/{dataset_id}?client_id={client.client_id}" From fcd1c4a880627c4771a9495a3f2743dbe2597c4c Mon Sep 17 00:00:00 2001 From: nupursharma-labellerr Date: Mon, 22 Dec 2025 22:30:07 +0530 Subject: [PATCH 03/11] [LABIMP-8413] Updating code as per Claude code comments --- labellerr/core/datasets/base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/labellerr/core/datasets/base.py b/labellerr/core/datasets/base.py index acade55..3c44793 100644 --- a/labellerr/core/datasets/base.py +++ b/labellerr/core/datasets/base.py @@ -12,6 +12,7 @@ from ..files import LabellerrFile from ..connectors import LabellerrConnection +from ..exceptions import InvalidDatasetError, InvalidDatasetIDError, LabellerrError if TYPE_CHECKING: from ..projects import LabellerrProject @@ -30,10 +31,12 @@ def _register(cls, data_type, dataset_class): def get_dataset(client: "LabellerrClient", dataset_id: str): """Get dataset from Labellerr API""" # Validate dataset_id format (should be a valid UUID) + if not dataset_id: + raise InvalidDatasetIDError("Dataset ID cannot be None or empty") try: uuid.UUID(dataset_id) - except (ValueError, AttributeError): - raise InvalidDatasetError(f"Invalid dataset ID format: {dataset_id}") + except (ValueError, TypeError): + raise InvalidDatasetIDError(f"Invalid dataset ID format: {dataset_id}") unique_id = str(uuid.uuid4()) url = ( From 46b4f2862029a1b4df298d1d9ff1b26de8442d5b Mon Sep 17 00:00:00 2001 From: nupursharma-labellerr Date: Mon, 22 Dec 2025 22:40:53 +0530 Subject: [PATCH 04/11] [LABIMP-8413] Updating code as per Claude code comments --- labellerr/core/datasets/base.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/labellerr/core/datasets/base.py b/labellerr/core/datasets/base.py index 3c44793..764d665 100644 --- a/labellerr/core/datasets/base.py +++ b/labellerr/core/datasets/base.py @@ -7,12 +7,11 @@ from typing import Dict, Any, Generator, TYPE_CHECKING from .. import constants -from ..exceptions import InvalidDatasetError, LabellerrError +from ..exceptions import InvalidDatasetError, LabellerrError, InvalidDatasetIDError from ..client import LabellerrClient from ..files import LabellerrFile from ..connectors import LabellerrConnection -from ..exceptions import InvalidDatasetError, InvalidDatasetIDError, LabellerrError if TYPE_CHECKING: from ..projects import LabellerrProject From 7ba059eefa04cd96dfa217d333bd03fe5cb7a304 Mon Sep 17 00:00:00 2001 From: nupursharma-labellerr Date: Wed, 7 Jan 2026 16:16:03 +0530 Subject: [PATCH 05/11] [LABIMP-8649] pytest for dataset creation, listing and deletion --- .../test_dataset_creation_integration.py | 616 ++++++++++++++++++ tests/unit/test_dataset_creation.py | 535 +++++++++++++++ 2 files changed, 1151 insertions(+) create mode 100644 tests/integration/test_dataset_creation_integration.py create mode 100644 tests/unit/test_dataset_creation.py diff --git a/tests/integration/test_dataset_creation_integration.py b/tests/integration/test_dataset_creation_integration.py new file mode 100644 index 0000000..7ada1fe --- /dev/null +++ b/tests/integration/test_dataset_creation_integration.py @@ -0,0 +1,616 @@ +""" +Integration tests for dataset creation and management. + +These tests make actual API calls to verify dataset creation, deletion, +and listing functionality works correctly with the Labellerr API. +""" + +import os +import time +import tempfile +from pathlib import Path + +import pytest + +from labellerr.client import LabellerrClient +from labellerr.core.datasets import ( + LabellerrDataset, + create_dataset_from_local, + create_dataset_from_connection, + delete_dataset, + list_datasets, +) +from labellerr.core.exceptions import ( + LabellerrError, + InvalidDatasetIDError, + InvalidDatasetError, +) +from labellerr.core.schemas import DatasetConfig, DataSetScope + + +# Module-level list to track all created datasets for cleanup +_created_datasets = [] + + +def register_dataset_for_cleanup(dataset_id: str, client: LabellerrClient): + """Register a dataset ID for cleanup at the end of test session""" + if dataset_id and dataset_id not in _created_datasets: + _created_datasets.append((dataset_id, client)) + + +def cleanup_all_datasets(): + """Clean up all registered datasets""" + print(f"\n\nCleaning up {len(_created_datasets)} created datasets...") + for dataset_id, client in _created_datasets: + try: + delete_dataset(client, dataset_id) + print(f" ✓ Deleted dataset: {dataset_id}") + except Exception as e: + print(f" ✗ Failed to delete dataset {dataset_id}: {e}") + _created_datasets.clear() + + +@pytest.fixture(scope="session", autouse=True) +def cleanup_datasets_on_exit(request): + """Automatically cleanup all created datasets at end of test session""" + yield + cleanup_all_datasets() + + +def get_test_images_from_env(num_images: int = 3) -> list: + """ + Get real test images from IMG_DATASET_PATH environment variable. + Returns a list of image file paths. Skips test if path doesn't exist or has no images. + """ + img_path = os.getenv("IMG_DATASET_PATH") + + if not img_path: + pytest.skip("IMG_DATASET_PATH not set in environment") + + img_path = Path(img_path) + + if not img_path.exists(): + pytest.skip(f"IMG_DATASET_PATH does not exist: {img_path}") + + if not img_path.is_dir(): + pytest.skip(f"IMG_DATASET_PATH is not a directory: {img_path}") + + # Find image files (jpg, jpeg, png) + image_extensions = {'.jpg', '.jpeg', '.png', '.gif', '.bmp'} + image_files = [ + f for f in img_path.iterdir() + if f.is_file() and f.suffix.lower() in image_extensions + ] + + if not image_files: + pytest.skip(f"No image files found in IMG_DATASET_PATH: {img_path}") + + if len(image_files) < num_images: + pytest.skip(f"Not enough images in IMG_DATASET_PATH. Found {len(image_files)}, need {num_images}") + + # Return first num_images files + return image_files[:num_images] + + +def skip_if_auth_error(e: Exception): + """Helper to skip tests when authentication fails or file uploads fail due to auth""" + error_str = str(e).lower() + # Check for direct auth errors + if "403" in str(e) or "not authorized" in error_str or "invalid api key" in error_str: + pytest.skip(f"API credentials invalid or expired: {e}") + # Check for file upload failures (which often hide auth errors in logs) + if "all file uploads failed" in error_str: + pytest.skip(f"File uploads failed (likely due to invalid credentials): {e}") + + +@pytest.mark.integration +class TestDatasetCreationIntegration: + """Integration tests for dataset creation""" + + def test_create_dataset_from_local_folder(self, integration_client, test_credentials): + """ + Comprehensive test: dataset creation, all properties validation, and property types. + Tests: + - Basic dataset creation from local folder + - All property accessors (name, data_type, files_count, status_code, etc.) + - Property types validation + - Files count accuracy + """ + # Get real test images from IMG_DATASET_PATH + test_images = get_test_images_from_env(num_images=3) + + # Copy images to a temporary folder for testing + with tempfile.TemporaryDirectory() as tmpdir: + import shutil + + # Copy real images to temp directory + for img_file in test_images: + shutil.copy2(img_file, tmpdir) + + # Verify files were copied + copied_files = list(Path(tmpdir).iterdir()) + assert len(copied_files) == 3, f"Expected 3 files, found {len(copied_files)}" + + dataset_config = DatasetConfig( + dataset_name=f"Test Local Dataset {int(time.time())}", + dataset_description="Integration test dataset from local folder with real images", + data_type="image", + ) + + dataset_id = None + + try: + # Create dataset from local folder + dataset = create_dataset_from_local( + client=integration_client, + dataset_config=dataset_config, + folder_to_upload=tmpdir, + ) + + dataset_id = dataset.dataset_id + # Register for cleanup + register_dataset_for_cleanup(dataset_id, integration_client) + + # Verify dataset was created + assert dataset is not None + assert dataset.dataset_id is not None + assert dataset.name == dataset_config.dataset_name + + # Test all property accessors + assert dataset.data_type == "image" + assert hasattr(dataset, 'files_count') + assert hasattr(dataset, 'status_code') + assert hasattr(dataset, 'description') + assert hasattr(dataset, 'created_at') + assert hasattr(dataset, 'created_by') + + # Validate property types + assert isinstance(dataset.dataset_id, str) + assert isinstance(dataset.data_type, str) + assert isinstance(dataset.files_count, int) + assert isinstance(dataset.status_code, int) + if dataset.name is not None: + assert isinstance(dataset.name, str) + + # Print properties for verification + print(f"\nDataset Properties:") + print(f" ID: {dataset.dataset_id}") + print(f" Name: {dataset.name}") + print(f" Data Type: {dataset.data_type}") + print(f" Files Count: {dataset.files_count}") + print(f" Status Code: {dataset.status_code}") + print(f" Description: {dataset.description}") + + # Wait for dataset processing + status = dataset.status() + assert status is not None + + # Clean up - delete the dataset + delete_result = delete_dataset(integration_client, dataset_id) + assert delete_result is not None + dataset_id = None + + except LabellerrError as e: + skip_if_auth_error(e) + if "500" in str(e) or "unavailable" in str(e).lower(): + pytest.skip(f"API unavailable: {e}") + else: + raise + finally: + if dataset_id: + try: + delete_dataset(integration_client, dataset_id) + except Exception: + pass + + def test_create_dataset_from_connection_with_existing_connection( + self, integration_client, test_credentials, test_project_ids + ): + """Test creating a dataset using an existing connection""" + # Skip if no connection ID available + connection_id = os.getenv("TEST_CONNECTION_ID") + if not connection_id: + pytest.skip("TEST_CONNECTION_ID not set in environment") + + dataset_config = DatasetConfig( + dataset_name=f"Test Connection Dataset {int(time.time())}", + dataset_description="Integration test dataset from connection", + data_type="image", + ) + + try: + # Create dataset from connection + dataset = create_dataset_from_connection( + client=integration_client, + dataset_config=dataset_config, + connection=connection_id, + path="test/path", + ) + + # Verify dataset was created + assert dataset is not None + assert dataset.dataset_id is not None + assert dataset.name == dataset_config.dataset_name + + # Clean up + delete_dataset(integration_client, dataset.dataset_id) + + except LabellerrError as e: + skip_if_auth_error(e) + if any(phrase in str(e).lower() for phrase in ["500", "unavailable", "not found"]): + pytest.skip(f"Test skipped due to: {e}") + else: + raise + + def test_create_dataset_with_multimodal_indexing( + self, integration_client, test_credentials + ): + """ + Comprehensive test: multimodal indexing and dataset deletion. + Tests dataset creation with multimodal indexing, then verifies deletion works correctly. + """ + # Get real test images + test_images = get_test_images_from_env(num_images=1) + + with tempfile.TemporaryDirectory() as tmpdir: + import shutil + + # Copy real image to temp directory + shutil.copy2(test_images[0], tmpdir) + + dataset_config = DatasetConfig( + dataset_name=f"Multimodal Test Dataset {int(time.time())}", + data_type="image", + multimodal_indexing=True, + ) + + dataset_id = None + + try: + # Create dataset with multimodal indexing + dataset = create_dataset_from_local( + client=integration_client, + dataset_config=dataset_config, + folder_to_upload=tmpdir, + ) + + assert dataset is not None + dataset_id = dataset.dataset_id + assert dataset_id is not None + # Register for cleanup + register_dataset_for_cleanup(dataset_id, integration_client) + + # Verify multimodal indexing can be enabled + result = dataset.enable_multimodal_indexing(is_multimodal=True) + assert result is not None + + # Test deletion + delete_result = delete_dataset(integration_client, dataset_id) + assert delete_result is not None + + # Mark as deleted (don't verify by fetching as API may return 500 errors) + dataset_id = None + + except LabellerrError as e: + skip_if_auth_error(e) + if "500" in str(e) or "unavailable" in str(e).lower(): + pytest.skip(f"API unavailable: {e}") + else: + raise + finally: + if dataset_id: + try: + delete_dataset(integration_client, dataset_id) + except Exception: + pass + + +@pytest.mark.integration +class TestDatasetValidationIntegration: + """Integration tests for dataset ID validation with real API""" + + def test_invalid_dataset_id_format_rejected(self, integration_client): + """Test that invalid dataset ID formats are rejected before API call""" + invalid_ids = [ + "invalid-id", + "not-a-uuid", + "05becc9c-e221-42ea-90f8-8d24031e2f3b1", # Extra character + "123456", + ] + + for invalid_id in invalid_ids: + with pytest.raises(InvalidDatasetIDError, match="Invalid dataset ID format"): + LabellerrDataset(integration_client, invalid_id) + + def test_valid_uuid_format_but_nonexistent_dataset(self, integration_client): + """Test that valid UUID format but non-existent dataset returns proper error""" + nonexistent_id = "00000000-0000-0000-0000-000000000000" + + try: + # This should fail because the dataset doesn't exist + with pytest.raises((InvalidDatasetError, LabellerrError)) as exc_info: + dataset = LabellerrDataset(integration_client, nonexistent_id) + + # Skip if we got auth error + if exc_info.value: + skip_if_auth_error(exc_info.value) + + # Verify error message mentions dataset not found + assert "not found" in str(exc_info.value).lower() or "dataset" in str(exc_info.value).lower() + + except Exception as e: + # If we get RetryError or 500 errors, that's expected for non-existent datasets + if "RetryError" in str(type(e).__name__) or "500" in str(e): + pass # Expected + else: + raise + + def test_empty_dataset_id_rejected(self, integration_client): + """Test that empty dataset_id is rejected""" + with pytest.raises(InvalidDatasetIDError, match="Dataset ID cannot be None or empty"): + LabellerrDataset(integration_client, "") + +@pytest.mark.integration +class TestDatasetDeletionIntegration: + """Integration tests for dataset deletion""" + + def test_delete_nonexistent_dataset(self, integration_client): + """Test deletion of non-existent dataset returns appropriate error""" + nonexistent_id = "00000000-0000-0000-0000-000000000001" + + try: + with pytest.raises(LabellerrError): + delete_dataset(integration_client, nonexistent_id) + except Exception as e: + # Some error is expected + assert "not found" in str(e).lower() or "error" in str(e).lower() + + +@pytest.mark.integration +class TestDatasetListingIntegration: + """Integration tests for dataset listing""" + + def test_list_datasets_client_scope(self, integration_client, test_credentials): + """Test listing datasets with client scope""" + try: + datasets = list(list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.client, + page_size=10, + )) + + # Should return a list (may be empty) + assert isinstance(datasets, list) + + # If datasets exist, verify structure + if datasets: + for dataset in datasets: + assert "dataset_id" in dataset + # May have other fields like name, data_type, etc. + + except LabellerrError as e: + skip_if_auth_error(e) + if "500" in str(e) or "unavailable" in str(e).lower(): + pytest.skip(f"API unavailable: {e}") + else: + raise + + def test_list_datasets_auto_pagination(self, integration_client, test_credentials): + """Test listing datasets with auto-pagination (page_size=-1)""" + try: + datasets = list(list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.client, + page_size=-1, # Auto-pagination + )) + + # Should return a list + assert isinstance(datasets, list) + + except LabellerrError as e: + skip_if_auth_error(e) + if "500" in str(e) or "unavailable" in str(e).lower(): + pytest.skip(f"API unavailable: {e}") + else: + raise + + def test_list_datasets_different_data_types(self, integration_client, test_credentials): + """Test listing datasets for different data types""" + data_types = ["image", "video", "document"] + + for data_type in data_types: + try: + datasets = list(list_datasets( + client=integration_client, + datatype=data_type, + scope=DataSetScope.client, + page_size=5, + )) + + assert isinstance(datasets, list) + + except LabellerrError as e: + skip_if_auth_error(e) + if "500" in str(e) or "unavailable" in str(e).lower(): + pytest.skip(f"API unavailable for {data_type}: {e}") + else: + raise + + def test_list_datasets_user_scope(self, integration_client, test_credentials): + """Test listing datasets with project scope""" + try: + datasets = list(list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.project, + page_size=10, + )) + + # Should return a list (may be empty) + assert isinstance(datasets, list) + + except LabellerrError as e: + skip_if_auth_error(e) + if "500" in str(e) or "unavailable" in str(e).lower(): + pytest.skip(f"API unavailable: {e}") + else: + raise + + +@pytest.mark.integration +class TestDatasetWorkflowIntegration: + """Integration tests for complete dataset workflows""" + + def test_dataset_update_operations(self, integration_client): + """ + Test dataset update operations: name, description, and metadata. + + NOTE: This test currently documents that update operations are NOT YET IMPLEMENTED. + When update functionality is added to the SDK, this test will validate it. + For now, it verifies that datasets can be created and their properties accessed. + """ + # Get real test images + test_images = get_test_images_from_env(num_images=1) + + with tempfile.TemporaryDirectory() as tmpdir: + import shutil + + # Copy real image to temp directory + shutil.copy2(test_images[0], tmpdir) + + dataset_config = DatasetConfig( + dataset_name=f"Update Test Dataset {int(time.time())}", + dataset_description="Original description for update testing", + data_type="image", + ) + + dataset_id = None + + try: + # Create dataset + dataset = create_dataset_from_local( + client=integration_client, + dataset_config=dataset_config, + folder_to_upload=tmpdir, + ) + + assert dataset is not None + dataset_id = dataset.dataset_id + assert dataset_id is not None + # Register for cleanup + register_dataset_for_cleanup(dataset_id, integration_client) + + # Verify original properties are accessible + assert dataset.name == dataset_config.dataset_name + assert dataset.data_type == "image" + + # Document what update operations are NOT YET IMPLEMENTED: + print(f"\n⚠ Update operations not yet implemented in SDK:") + print(f" - update_name() - method does not exist") + print(f" - update_description() - method does not exist") + print(f" - update_metadata() - method does not exist") + print(f" - add_files() - method does not exist") + print(f" - remove_files() - method does not exist") + + # Verify that these methods don't exist (expected) + assert not hasattr(dataset, 'update_name'), "update_name unexpectedly exists" + assert not hasattr(dataset, 'update_description'), "update_description unexpectedly exists" + assert not hasattr(dataset, 'update_metadata'), "update_metadata unexpectedly exists" + assert not hasattr(dataset, 'add_files'), "add_files unexpectedly exists" + assert not hasattr(dataset, 'remove_files'), "remove_files unexpectedly exists" + + # Clean up + delete_result = delete_dataset(integration_client, dataset_id) + assert delete_result is not None + dataset_id = None + + except LabellerrError as e: + skip_if_auth_error(e) + if "500" in str(e) or "unavailable" in str(e).lower(): + pytest.skip(f"API unavailable: {e}") + else: + raise + finally: + if dataset_id: + try: + delete_dataset(integration_client, dataset_id) + except Exception: + pass + + def test_complete_dataset_lifecycle(self, integration_client, test_credentials): + """Test complete dataset lifecycle: create, fetch, list, and delete""" + # Get real test images + test_images = get_test_images_from_env(num_images=2) + + with tempfile.TemporaryDirectory() as tmpdir: + import shutil + + # Copy real images to temp directory + for img_file in test_images: + shutil.copy2(img_file, tmpdir) + + dataset_config = DatasetConfig( + dataset_name=f"Lifecycle Test Dataset {int(time.time())}", + dataset_description="Testing complete lifecycle", + data_type="image", + ) + + dataset_id = None + + try: + # Step 1: Create dataset + dataset = create_dataset_from_local( + client=integration_client, + dataset_config=dataset_config, + folder_to_upload=tmpdir, + ) + + assert dataset is not None + dataset_id = dataset.dataset_id + assert dataset_id is not None + # Register for cleanup + register_dataset_for_cleanup(dataset_id, integration_client) + + # Step 2: Fetch dataset by ID + fetched_dataset = LabellerrDataset(integration_client, dataset_id) + assert fetched_dataset.dataset_id == dataset_id + assert fetched_dataset.name == dataset_config.dataset_name + + # Step 3: Check dataset status + status = fetched_dataset.status() + assert status is not None + assert "status_code" in status + + # Step 4: List datasets and verify our dataset is in the list + datasets = list(list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.client, + page_size=20, + )) + + dataset_ids = [d.get("dataset_id") for d in datasets] + # Our dataset might or might not be in the first page + # So we just verify the list operation worked + + # Step 5: Delete dataset + delete_result = delete_dataset(integration_client, dataset_id) + assert delete_result is not None + + dataset_id = None # Mark as deleted + + except LabellerrError as e: + skip_if_auth_error(e) + if "500" in str(e) or "unavailable" in str(e).lower(): + pytest.skip(f"API unavailable: {e}") + else: + raise + finally: + # Cleanup: ensure dataset is deleted even if test fails + if dataset_id: + try: + delete_dataset(integration_client, dataset_id) + except Exception: + pass # Ignore cleanup errors diff --git a/tests/unit/test_dataset_creation.py b/tests/unit/test_dataset_creation.py new file mode 100644 index 0000000..38cc2da --- /dev/null +++ b/tests/unit/test_dataset_creation.py @@ -0,0 +1,535 @@ +""" +Comprehensive unit tests for dataset creation and management. + +This module tests the dataset creation, deletion, and listing functionality +including validation, error handling, and edge cases. +""" + +from unittest.mock import Mock, patch, MagicMock +import pytest + +from labellerr.core.datasets import ( + create_dataset_from_connection, + create_dataset_from_local, + delete_dataset, + list_datasets, +) +from labellerr.core.datasets.base import LabellerrDataset, LabellerrDatasetMeta +from labellerr.core.exceptions import ( + LabellerrError, + InvalidDatasetIDError, + InvalidDatasetError, +) +from labellerr.core.schemas import DatasetConfig, DataSetScope + + +@pytest.mark.unit +class TestDatasetCreation: + """Test dataset creation functions""" + + def test_create_dataset_from_connection_with_string_connection_id(self, client): + """Test dataset creation with string connection_id""" + dataset_config = DatasetConfig( + dataset_name="Test Dataset", + data_type="image", + ) + + mock_response = { + "response": {"dataset_id": "550e8400-e29b-41d4-a716-446655440000", "data_type": "image"} + } + + with patch.object(client, "make_request", return_value=mock_response): + with patch( + "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", + return_value={"dataset_id": "550e8400-e29b-41d4-a716-446655440000", "data_type": "image"}, + ): + dataset = create_dataset_from_connection( + client=client, + dataset_config=dataset_config, + connection="test-connection-id", + path="s3://bucket/path", + ) + + assert dataset is not None + assert dataset.dataset_id == "550e8400-e29b-41d4-a716-446655440000" + + def test_create_dataset_from_connection_with_connection_object(self, client): + """Test dataset creation with LabellerrConnection object""" + from labellerr.core.connectors import LabellerrConnection + + dataset_config = DatasetConfig( + dataset_name="Test Dataset", + data_type="video", + ) + + # Create a proper mock connection with required attributes + mock_connection = Mock(spec=LabellerrConnection) + mock_connection.connection_id = "test-connection-id" + + mock_response = { + "response": {"dataset_id": "550e8400-e29b-41d4-a716-446655440001", "data_type": "video"} + } + + with patch.object(client, "make_request", return_value=mock_response): + with patch( + "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", + return_value={"dataset_id": "550e8400-e29b-41d4-a716-446655440001", "data_type": "video"}, + ): + dataset = create_dataset_from_connection( + client=client, + dataset_config=dataset_config, + connection=mock_connection, + path="gs://bucket/path", + ) + + assert dataset is not None + + def test_create_dataset_from_local_with_files_list(self, client): + """Test creating dataset from local files list""" + dataset_config = DatasetConfig( + dataset_name="Local Files Dataset", + data_type="image", + ) + + files_to_upload = ["/path/to/file1.jpg", "/path/to/file2.jpg"] + + with patch("labellerr.core.datasets.upload_files", return_value="local-connection-id"): + with patch("labellerr.core.datasets.create_dataset_from_connection") as mock_create: + mock_dataset = Mock() + mock_dataset.dataset_id = "test-dataset-id" + mock_create.return_value = mock_dataset + + dataset = create_dataset_from_local( + client=client, + dataset_config=dataset_config, + files_to_upload=files_to_upload, + ) + + assert dataset is not None + mock_create.assert_called_once() + + def test_create_dataset_from_local_with_folder(self, client): + """Test creating dataset from local folder""" + dataset_config = DatasetConfig( + dataset_name="Local Folder Dataset", + data_type="document", + ) + + folder_path = "/path/to/documents" + + with patch( + "labellerr.core.datasets.upload_folder_files_to_dataset", + return_value={"connection_id": "folder-connection-id", "status": "success"} + ): + with patch("labellerr.core.datasets.create_dataset_from_connection") as mock_create: + mock_dataset = Mock() + mock_dataset.dataset_id = "test-dataset-id" + mock_create.return_value = mock_dataset + + dataset = create_dataset_from_local( + client=client, + dataset_config=dataset_config, + folder_to_upload=folder_path, + ) + + assert dataset is not None + + def test_create_dataset_from_local_no_source(self, client): + """Test error when no files or folder provided""" + dataset_config = DatasetConfig( + dataset_name="Invalid Dataset", + data_type="image", + ) + + with pytest.raises(LabellerrError, match="No files or folder to upload provided"): + create_dataset_from_local( + client=client, + dataset_config=dataset_config, + ) + + def test_create_dataset_with_multimodal_indexing(self, client): + """Test creating dataset with multimodal indexing enabled""" + dataset_config = DatasetConfig( + dataset_name="Multimodal Dataset", + data_type="image", + multimodal_indexing=True, + ) + + mock_response = { + "response": {"dataset_id": "test-dataset-id", "data_type": "image"} + } + + with patch.object(client, "make_request", return_value=mock_response) as mock_request: + with patch( + "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", + return_value={"dataset_id": "test-dataset-id", "data_type": "image"}, + ): + dataset = create_dataset_from_connection( + client=client, + dataset_config=dataset_config, + connection="test-connection", + path="s3://bucket/path", + ) + + # Verify multimodal_indexing was passed in the request + call_args = mock_request.call_args + assert "data" in call_args.kwargs + import json + payload = json.loads(call_args.kwargs["data"]) + assert payload["es_multimodal_index"] is True + + +@pytest.mark.unit +class TestDatasetDeletion: + """Test dataset deletion functionality""" + + def test_delete_dataset_success(self, client): + """Test successful dataset deletion""" + dataset_id = "550e8400-e29b-41d4-a716-446655440000" + + mock_response = { + "response": {"status": "deleted", "dataset_id": dataset_id} + } + + with patch.object(client, "make_request", return_value=mock_response): + result = delete_dataset(client, dataset_id) + + assert result is not None + assert result["response"]["status"] == "deleted" + + def test_delete_dataset_invalid_id(self, client): + """Test deletion with invalid dataset ID""" + invalid_id = "not-a-valid-uuid" + + # The deletion function doesn't validate UUID format before making request + # So it will make the API call which should fail + with patch.object(client, "make_request", side_effect=LabellerrError("Invalid dataset ID")): + with pytest.raises(LabellerrError): + delete_dataset(client, invalid_id) + + def test_delete_nonexistent_dataset(self, client): + """Test deletion of non-existent dataset""" + dataset_id = "00000000-0000-0000-0000-000000000000" + + with patch.object(client, "make_request", side_effect=LabellerrError("Dataset not found")): + with pytest.raises(LabellerrError, match="Dataset not found"): + delete_dataset(client, dataset_id) + + +@pytest.mark.unit +class TestDatasetListing: + """Test dataset listing functionality""" + + def test_list_datasets_single_page(self, client): + """Test listing datasets with single page""" + mock_response = { + "response": { + "datasets": [ + {"dataset_id": "id1", "name": "Dataset 1"}, + {"dataset_id": "id2", "name": "Dataset 2"}, + ], + "has_more": False, + } + } + + with patch.object(client, "make_request", return_value=mock_response): + datasets = list(list_datasets( + client=client, + datatype="image", + scope=DataSetScope.client, + page_size=10, + )) + + assert len(datasets) == 2 + assert datasets[0]["dataset_id"] == "id1" + + def test_list_datasets_auto_pagination(self, client): + """Test listing datasets with auto-pagination (page_size=-1)""" + # Mock multiple pages + mock_responses = [ + { + "response": { + "datasets": [{"dataset_id": f"id{i}"} for i in range(10)], + "has_more": True, + "last_dataset_id": "id9", + } + }, + { + "response": { + "datasets": [{"dataset_id": f"id{i}"} for i in range(10, 15)], + "has_more": False, + } + }, + ] + + with patch.object(client, "make_request", side_effect=mock_responses): + datasets = list(list_datasets( + client=client, + datatype="image", + scope=DataSetScope.client, + page_size=-1, # Auto-pagination + )) + + assert len(datasets) == 15 + assert datasets[0]["dataset_id"] == "id0" + assert datasets[-1]["dataset_id"] == "id14" + + def test_list_datasets_empty_result(self, client): + """Test listing datasets when no datasets exist""" + mock_response = { + "response": { + "datasets": [], + "has_more": False, + } + } + + with patch.object(client, "make_request", return_value=mock_response): + datasets = list(list_datasets( + client=client, + datatype="video", + scope="user", # Use string instead of enum + page_size=10, + )) + + assert len(datasets) == 0 + + def test_list_datasets_with_last_dataset_id(self, client): + """Test manual pagination with last_dataset_id""" + mock_response = { + "response": { + "datasets": [ + {"dataset_id": "id11", "name": "Dataset 11"}, + {"dataset_id": "id12", "name": "Dataset 12"}, + ], + "has_more": True, + "last_dataset_id": "id12", + } + } + + with patch.object(client, "make_request", return_value=mock_response) as mock_request: + datasets = list(list_datasets( + client=client, + datatype="document", + scope=DataSetScope.client, + page_size=10, + last_dataset_id="id10", + )) + + assert len(datasets) == 2 + # Verify last_dataset_id was included in URL + call_args = mock_request.call_args + assert "last_dataset_id=id10" in call_args[0][1] # URL is second positional arg + + +@pytest.mark.unit +class TestDatasetValidation: + """Test dataset ID validation""" + + def test_valid_uuid_format(self, client): + """Test that valid UUID formats pass validation""" + valid_uuids = [ + "550e8400-e29b-41d4-a716-446655440000", + "1c8b2a05-0321-44fd-91e3-2ea911382cf9", + "00000000-0000-0000-0000-000000000000", + ] + + for dataset_id in valid_uuids: + # Validation should pass, but API call will fail (mocked) + with patch.object(client, "make_request", side_effect=LabellerrError("API error")): + with pytest.raises(LabellerrError): + LabellerrDatasetMeta.get_dataset(client, dataset_id) + + def test_invalid_uuid_format_rejected(self, client): + """Test that invalid UUID formats are rejected before API call""" + invalid_ids = [ + "invalid-id", + "not-a-uuid", + "123456", + "05becc9c-e221-42ea-90f8-8d24031e2f3b1", # Extra character + "05becc9c-e221-42ea-90f8", # Too short + ] + + for dataset_id in invalid_ids: + with pytest.raises(InvalidDatasetIDError, match="Invalid dataset ID format"): + LabellerrDatasetMeta.get_dataset(client, dataset_id) + + def test_empty_dataset_id(self, client): + """Test that empty dataset_id is rejected""" + with pytest.raises(InvalidDatasetIDError, match="Dataset ID cannot be None or empty"): + LabellerrDatasetMeta.get_dataset(client, "") + + def test_none_dataset_id(self, client): + """Test that None dataset_id is rejected""" + with pytest.raises((InvalidDatasetIDError, TypeError)): + LabellerrDatasetMeta.get_dataset(client, None) + + def test_non_string_dataset_id(self, client): + """Test that non-string dataset_id is rejected""" + with pytest.raises((InvalidDatasetIDError, TypeError, AttributeError)): + LabellerrDatasetMeta.get_dataset(client, 12345) + + +@pytest.mark.unit +class TestDatasetErrorHandling: + """Test error handling in dataset operations""" + + def test_other_exceptions_propagate(self, client): + """Test that exceptions from API are propagated""" + dataset_id = "550e8400-e29b-41d4-a716-446655440000" + + # Simulate an API error + api_error = LabellerrError("API error occurred") + + with patch.object(client, "make_request", side_effect=api_error): + with pytest.raises(LabellerrError, match="API error occurred"): + LabellerrDatasetMeta.get_dataset(client, dataset_id) + + def test_custom_exceptions_not_converted(self, client): + """Test that non-API exceptions are not converted""" + dataset_id = "550e8400-e29b-41d4-a716-446655440000" + + # Simulate a different kind of error + custom_error = ValueError("Custom error") + + with patch.object(client, "make_request", side_effect=custom_error): + with pytest.raises(ValueError, match="Custom error"): + LabellerrDatasetMeta.get_dataset(client, dataset_id) + + +@pytest.mark.unit +class TestDatasetConfig: + """Test DatasetConfig schema validation""" + + def test_valid_dataset_config(self): + """Test creating valid dataset config""" + config = DatasetConfig( + dataset_name="Test Dataset", + data_type="image", + ) + + assert config.dataset_name == "Test Dataset" + assert config.data_type == "image" + # dataset_description defaults to empty string, not None + assert config.dataset_description == "" + assert config.multimodal_indexing is False + + def test_dataset_config_with_all_fields(self): + """Test dataset config with all fields""" + config = DatasetConfig( + dataset_name="Full Dataset", + data_type="video", + dataset_description="A test dataset", + multimodal_indexing=True, + ) + + assert config.dataset_name == "Full Dataset" + assert config.data_type == "video" + assert config.dataset_description == "A test dataset" + assert config.multimodal_indexing is True + + def test_dataset_config_with_different_data_types(self): + """Test dataset config with various data types""" + data_types = ["image", "video", "audio", "document", "text"] + + for data_type in data_types: + config = DatasetConfig( + dataset_name=f"{data_type.capitalize()} Dataset", + data_type=data_type, + ) + assert config.data_type == data_type + + +@pytest.mark.unit +class TestDatasetProperties: + """Test dataset property access""" + + def test_dataset_properties_access(self, client): + """Test accessing dataset properties""" + dataset_id = "550e8400-e29b-41d4-a716-446655440000" + + mock_dataset_data = { + "dataset_id": dataset_id, + "name": "Test Dataset", + "description": "Test Description", + "data_type": "image", + "files_count": 42, + "status_code": 300, + "created_at": "2024-01-01T00:00:00Z", + "created_by": "test@example.com", + } + + with patch( + "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", + return_value=mock_dataset_data, + ): + dataset = LabellerrDataset(client, dataset_id) + + # Test all properties + assert dataset.dataset_id == dataset_id + assert dataset.name == "Test Dataset" + assert dataset.description == "Test Description" + assert dataset.data_type == "image" + assert dataset.files_count == 42 + assert dataset.status_code == 300 + assert dataset.created_at == "2024-01-01T00:00:00Z" + assert dataset.created_by == "test@example.com" + + def test_dataset_properties_defaults(self, client): + """Test dataset property defaults when data is missing""" + dataset_id = "550e8400-e29b-41d4-a716-446655440000" + + # Minimal dataset data + mock_dataset_data = { + "dataset_id": dataset_id, + "data_type": "image", + } + + with patch( + "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", + return_value=mock_dataset_data, + ): + dataset = LabellerrDataset(client, dataset_id) + + # Test defaults + assert dataset.dataset_id == dataset_id + assert dataset.data_type == "image" + assert dataset.files_count == 0 # Default + assert dataset.status_code == 501 # Default + + def test_dataset_status_property(self, client): + """Test dataset status_code property returns default when missing""" + dataset_id = "550e8400-e29b-41d4-a716-446655440000" + + mock_dataset_data = { + "dataset_id": dataset_id, + "data_type": "image", + # status_code not provided + } + + with patch( + "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", + return_value=mock_dataset_data, + ): + dataset = LabellerrDataset(client, dataset_id) + + # Should return default 501 when not found + assert dataset.status_code == 501 + + def test_dataset_files_count_zero_default(self, client): + """Test dataset files_count returns 0 when not provided""" + dataset_id = "550e8400-e29b-41d4-a716-446655440000" + + mock_dataset_data = { + "dataset_id": dataset_id, + "data_type": "video", + # files_count not provided + } + + with patch( + "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", + return_value=mock_dataset_data, + ): + dataset = LabellerrDataset(client, dataset_id) + + # Should return 0 when not found + assert dataset.files_count == 0 From 97c9167453b18ac5a41bdb87f762d7850c508a21 Mon Sep 17 00:00:00 2001 From: nupursharma-labellerr Date: Wed, 7 Jan 2026 22:23:55 +0530 Subject: [PATCH 06/11] [LABIMP-8649]: Claude PR comments --- .../test_dataset_creation_integration.py | 227 ++++++++---------- 1 file changed, 101 insertions(+), 126 deletions(-) diff --git a/tests/integration/test_dataset_creation_integration.py b/tests/integration/test_dataset_creation_integration.py index 7ada1fe..25fb569 100644 --- a/tests/integration/test_dataset_creation_integration.py +++ b/tests/integration/test_dataset_creation_integration.py @@ -34,8 +34,10 @@ def register_dataset_for_cleanup(dataset_id: str, client: LabellerrClient): """Register a dataset ID for cleanup at the end of test session""" - if dataset_id and dataset_id not in _created_datasets: + # Check if dataset_id already exists in the list of tuples + if dataset_id and dataset_id not in [d[0] for d in _created_datasets]: _created_datasets.append((dataset_id, client)) + print(f" → Registered dataset {dataset_id} for cleanup") def cleanup_all_datasets(): @@ -103,11 +105,42 @@ def skip_if_auth_error(e: Exception): pytest.skip(f"File uploads failed (likely due to invalid credentials): {e}") +def handle_api_errors(func): + """ + Decorator to handle common API errors in integration tests. + + Automatically: + - Skips tests on auth errors (403, invalid credentials) + - Skips tests on API unavailability (500, 503) + - Propagates other errors for proper failure reporting + + Usage: + @handle_api_errors + def test_something(self, integration_client): + # test code + """ + from functools import wraps + + @wraps(func) + def wrapper(*args, **kwargs): + try: + return func(*args, **kwargs) + except LabellerrError as e: + skip_if_auth_error(e) + if "500" in str(e) or "unavailable" in str(e).lower(): + pytest.skip(f"API unavailable: {e}") + else: + raise + + return wrapper + + @pytest.mark.integration class TestDatasetCreationIntegration: """Integration tests for dataset creation""" - def test_create_dataset_from_local_folder(self, integration_client, test_credentials): + @handle_api_errors + def test_create_dataset_from_local_folder(self, integration_client): """ Comprehensive test: dataset creation, all properties validation, and property types. Tests: @@ -190,12 +223,6 @@ def test_create_dataset_from_local_folder(self, integration_client, test_credent assert delete_result is not None dataset_id = None - except LabellerrError as e: - skip_if_auth_error(e) - if "500" in str(e) or "unavailable" in str(e).lower(): - pytest.skip(f"API unavailable: {e}") - else: - raise finally: if dataset_id: try: @@ -203,6 +230,7 @@ def test_create_dataset_from_local_folder(self, integration_client, test_credent except Exception: pass + @handle_api_errors def test_create_dataset_from_connection_with_existing_connection( self, integration_client, test_credentials, test_project_ids ): @@ -218,33 +246,24 @@ def test_create_dataset_from_connection_with_existing_connection( data_type="image", ) - try: - # Create dataset from connection - dataset = create_dataset_from_connection( - client=integration_client, - dataset_config=dataset_config, - connection=connection_id, - path="test/path", - ) - - # Verify dataset was created - assert dataset is not None - assert dataset.dataset_id is not None - assert dataset.name == dataset_config.dataset_name + # Create dataset from connection + dataset = create_dataset_from_connection( + client=integration_client, + dataset_config=dataset_config, + connection=connection_id, + path="test/path", + ) - # Clean up - delete_dataset(integration_client, dataset.dataset_id) + # Verify dataset was created + assert dataset is not None + assert dataset.dataset_id is not None + assert dataset.name == dataset_config.dataset_name - except LabellerrError as e: - skip_if_auth_error(e) - if any(phrase in str(e).lower() for phrase in ["500", "unavailable", "not found"]): - pytest.skip(f"Test skipped due to: {e}") - else: - raise + # Clean up + delete_dataset(integration_client, dataset.dataset_id) - def test_create_dataset_with_multimodal_indexing( - self, integration_client, test_credentials - ): + @handle_api_errors + def test_create_dataset_with_multimodal_indexing(self, integration_client): """ Comprehensive test: multimodal indexing and dataset deletion. Tests dataset creation with multimodal indexing, then verifies deletion works correctly. @@ -291,12 +310,6 @@ def test_create_dataset_with_multimodal_indexing( # Mark as deleted (don't verify by fetching as API may return 500 errors) dataset_id = None - except LabellerrError as e: - skip_if_auth_error(e) - if "500" in str(e) or "unavailable" in str(e).lower(): - pytest.skip(f"API unavailable: {e}") - else: - raise finally: if dataset_id: try: @@ -370,99 +383,72 @@ def test_delete_nonexistent_dataset(self, integration_client): class TestDatasetListingIntegration: """Integration tests for dataset listing""" - def test_list_datasets_client_scope(self, integration_client, test_credentials): + @handle_api_errors + def test_list_datasets_client_scope(self, integration_client): """Test listing datasets with client scope""" - try: - datasets = list(list_datasets( - client=integration_client, - datatype="image", - scope=DataSetScope.client, - page_size=10, - )) - - # Should return a list (may be empty) - assert isinstance(datasets, list) - - # If datasets exist, verify structure - if datasets: - for dataset in datasets: - assert "dataset_id" in dataset - # May have other fields like name, data_type, etc. - - except LabellerrError as e: - skip_if_auth_error(e) - if "500" in str(e) or "unavailable" in str(e).lower(): - pytest.skip(f"API unavailable: {e}") - else: - raise - - def test_list_datasets_auto_pagination(self, integration_client, test_credentials): + datasets = list(list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.client, + page_size=10, + )) + + # Should return a list (may be empty) + assert isinstance(datasets, list) + + # If datasets exist, verify structure + if datasets: + for dataset in datasets: + assert "dataset_id" in dataset + # May have other fields like name, data_type, etc. + + @handle_api_errors + def test_list_datasets_auto_pagination(self, integration_client): """Test listing datasets with auto-pagination (page_size=-1)""" - try: - datasets = list(list_datasets( - client=integration_client, - datatype="image", - scope=DataSetScope.client, - page_size=-1, # Auto-pagination - )) - - # Should return a list - assert isinstance(datasets, list) - - except LabellerrError as e: - skip_if_auth_error(e) - if "500" in str(e) or "unavailable" in str(e).lower(): - pytest.skip(f"API unavailable: {e}") - else: - raise - - def test_list_datasets_different_data_types(self, integration_client, test_credentials): + datasets = list(list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.client, + page_size=-1, # Auto-pagination + )) + + # Should return a list + assert isinstance(datasets, list) + + @handle_api_errors + def test_list_datasets_different_data_types(self, integration_client): """Test listing datasets for different data types""" data_types = ["image", "video", "document"] for data_type in data_types: - try: - datasets = list(list_datasets( - client=integration_client, - datatype=data_type, - scope=DataSetScope.client, - page_size=5, - )) - - assert isinstance(datasets, list) - - except LabellerrError as e: - skip_if_auth_error(e) - if "500" in str(e) or "unavailable" in str(e).lower(): - pytest.skip(f"API unavailable for {data_type}: {e}") - else: - raise - - def test_list_datasets_user_scope(self, integration_client, test_credentials): - """Test listing datasets with project scope""" - try: datasets = list(list_datasets( client=integration_client, - datatype="image", - scope=DataSetScope.project, - page_size=10, + datatype=data_type, + scope=DataSetScope.client, + page_size=5, )) - # Should return a list (may be empty) assert isinstance(datasets, list) - except LabellerrError as e: - skip_if_auth_error(e) - if "500" in str(e) or "unavailable" in str(e).lower(): - pytest.skip(f"API unavailable: {e}") - else: - raise + @handle_api_errors + def test_list_datasets_project_scope(self, integration_client): + """Test listing datasets with project scope""" + datasets = list(list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.project, + page_size=10, + )) + + # Should return a list (may be empty) + assert isinstance(datasets, list) @pytest.mark.integration class TestDatasetWorkflowIntegration: """Integration tests for complete dataset workflows""" + @handle_api_errors def test_dataset_update_operations(self, integration_client): """ Test dataset update operations: name, description, and metadata. @@ -526,12 +512,6 @@ def test_dataset_update_operations(self, integration_client): assert delete_result is not None dataset_id = None - except LabellerrError as e: - skip_if_auth_error(e) - if "500" in str(e) or "unavailable" in str(e).lower(): - pytest.skip(f"API unavailable: {e}") - else: - raise finally: if dataset_id: try: @@ -539,7 +519,8 @@ def test_dataset_update_operations(self, integration_client): except Exception: pass - def test_complete_dataset_lifecycle(self, integration_client, test_credentials): + @handle_api_errors + def test_complete_dataset_lifecycle(self, integration_client): """Test complete dataset lifecycle: create, fetch, list, and delete""" # Get real test images test_images = get_test_images_from_env(num_images=2) @@ -601,12 +582,6 @@ def test_complete_dataset_lifecycle(self, integration_client, test_credentials): dataset_id = None # Mark as deleted - except LabellerrError as e: - skip_if_auth_error(e) - if "500" in str(e) or "unavailable" in str(e).lower(): - pytest.skip(f"API unavailable: {e}") - else: - raise finally: # Cleanup: ensure dataset is deleted even if test fails if dataset_id: From b7dcb257e585b33813cfcbe98eda5edc3663c4ae Mon Sep 17 00:00:00 2001 From: nupursharma-labellerr Date: Fri, 9 Jan 2026 16:58:04 +0530 Subject: [PATCH 07/11] [LABIMP-8649]: Incorporating code review comments --- labellerr/core/datasets/__init__.py | 9 +- labellerr/core/datasets/base.py | 19 +- .../test_dataset_creation_integration.py | 549 +++++++++--------- 3 files changed, 293 insertions(+), 284 deletions(-) diff --git a/labellerr/core/datasets/__init__.py b/labellerr/core/datasets/__init__.py index f02bfdf..dca9185 100644 --- a/labellerr/core/datasets/__init__.py +++ b/labellerr/core/datasets/__init__.py @@ -121,14 +121,17 @@ def delete_dataset(client: "LabellerrClient", dataset_id: str): :raises LabellerrError: If the deletion fails """ unique_id = str(uuid.uuid4()) - url = f"{constants.BASE_URL}/datasets/{dataset_id}/delete?client_id={client.client_id}&uuid={unique_id}" + url = f"{constants.BASE_URL}/datasets/delete?client_id={client.client_id}&uuid={unique_id}" - return client.make_request( - "DELETE", + response = client.make_request( + "POST", url, extra_headers={"content-type": "application/json"}, request_id=unique_id, + json={"dataset_id": dataset_id}, ) + # Return the whole response since response.get("response") might be None + return response def list_datasets( diff --git a/labellerr/core/datasets/base.py b/labellerr/core/datasets/base.py index 764d665..551917c 100644 --- a/labellerr/core/datasets/base.py +++ b/labellerr/core/datasets/base.py @@ -4,7 +4,7 @@ import logging import uuid from abc import ABCMeta -from typing import Dict, Any, Generator, TYPE_CHECKING +from typing import Dict, Any, Generator, Optional, TYPE_CHECKING from .. import constants from ..exceptions import InvalidDatasetError, LabellerrError, InvalidDatasetIDError @@ -113,14 +113,19 @@ def status_code(self): def data_type(self): return self.__dataset_data.get("data_type") - def status(self) -> Dict[str, Any]: + def status( + self, + interval: float = 2.0, + timeout: Optional[float] = None, + max_retries: Optional[int] = None, + ) -> Dict[str, Any]: """ Poll dataset status until completion or timeout. Args: interval: Time in seconds between status checks (default: 2.0) - timeout: Maximum time in seconds to poll before giving up - max_retries: Maximum number of retries before giving up + timeout: Maximum time in seconds to poll before giving up (default: None - no timeout) + max_retries: Maximum number of retries before giving up (default: None - no retry limit) Returns: Final dataset data with status information @@ -177,9 +182,9 @@ def on_success(dataset_data): return poll( function=get_dataset_status, condition=is_completed, - interval=2.0, - timeout=None, - max_retries=None, + interval=interval, + timeout=timeout, + max_retries=max_retries, on_success=on_success, ) diff --git a/tests/integration/test_dataset_creation_integration.py b/tests/integration/test_dataset_creation_integration.py index 25fb569..cbc23ff 100644 --- a/tests/integration/test_dataset_creation_integration.py +++ b/tests/integration/test_dataset_creation_integration.py @@ -6,11 +6,13 @@ """ import os +import re import time import tempfile from pathlib import Path import pytest +import requests.exceptions from labellerr.client import LabellerrClient from labellerr.core.datasets import ( @@ -28,35 +30,135 @@ from labellerr.core.schemas import DatasetConfig, DataSetScope -# Module-level list to track all created datasets for cleanup -_created_datasets = [] +def enhance_api_error(error: Exception, context: str) -> str: + """ + Enhance API error messages with context for better CI diagnostics. + + Args: + error: The original exception + context: Description of what operation was being performed + + Returns: + Enhanced error message with API details + """ + error_msg = str(error) + + # Check for HTML error responses (API returning error pages) + if "" in error_msg or " list: @@ -94,53 +196,11 @@ def get_test_images_from_env(num_images: int = 3) -> list: return image_files[:num_images] -def skip_if_auth_error(e: Exception): - """Helper to skip tests when authentication fails or file uploads fail due to auth""" - error_str = str(e).lower() - # Check for direct auth errors - if "403" in str(e) or "not authorized" in error_str or "invalid api key" in error_str: - pytest.skip(f"API credentials invalid or expired: {e}") - # Check for file upload failures (which often hide auth errors in logs) - if "all file uploads failed" in error_str: - pytest.skip(f"File uploads failed (likely due to invalid credentials): {e}") - - -def handle_api_errors(func): - """ - Decorator to handle common API errors in integration tests. - - Automatically: - - Skips tests on auth errors (403, invalid credentials) - - Skips tests on API unavailability (500, 503) - - Propagates other errors for proper failure reporting - - Usage: - @handle_api_errors - def test_something(self, integration_client): - # test code - """ - from functools import wraps - - @wraps(func) - def wrapper(*args, **kwargs): - try: - return func(*args, **kwargs) - except LabellerrError as e: - skip_if_auth_error(e) - if "500" in str(e) or "unavailable" in str(e).lower(): - pytest.skip(f"API unavailable: {e}") - else: - raise - - return wrapper - - @pytest.mark.integration class TestDatasetCreationIntegration: """Integration tests for dataset creation""" - @handle_api_errors - def test_create_dataset_from_local_folder(self, integration_client): + def test_create_dataset_from_local_folder(self, integration_client, cleanup_datasets): """ Comprehensive test: dataset creation, all properties validation, and property types. Tests: @@ -170,67 +230,50 @@ def test_create_dataset_from_local_folder(self, integration_client): data_type="image", ) - dataset_id = None - - try: - # Create dataset from local folder - dataset = create_dataset_from_local( - client=integration_client, - dataset_config=dataset_config, - folder_to_upload=tmpdir, - ) - - dataset_id = dataset.dataset_id - # Register for cleanup - register_dataset_for_cleanup(dataset_id, integration_client) - - # Verify dataset was created - assert dataset is not None - assert dataset.dataset_id is not None - assert dataset.name == dataset_config.dataset_name - - # Test all property accessors - assert dataset.data_type == "image" - assert hasattr(dataset, 'files_count') - assert hasattr(dataset, 'status_code') - assert hasattr(dataset, 'description') - assert hasattr(dataset, 'created_at') - assert hasattr(dataset, 'created_by') - - # Validate property types - assert isinstance(dataset.dataset_id, str) - assert isinstance(dataset.data_type, str) - assert isinstance(dataset.files_count, int) - assert isinstance(dataset.status_code, int) - if dataset.name is not None: - assert isinstance(dataset.name, str) - - # Print properties for verification - print(f"\nDataset Properties:") - print(f" ID: {dataset.dataset_id}") - print(f" Name: {dataset.name}") - print(f" Data Type: {dataset.data_type}") - print(f" Files Count: {dataset.files_count}") - print(f" Status Code: {dataset.status_code}") - print(f" Description: {dataset.description}") - - # Wait for dataset processing - status = dataset.status() - assert status is not None - - # Clean up - delete the dataset - delete_result = delete_dataset(integration_client, dataset_id) - assert delete_result is not None - dataset_id = None + # Create dataset from local folder + dataset = create_dataset_from_local( + client=integration_client, + dataset_config=dataset_config, + folder_to_upload=tmpdir, + ) - finally: - if dataset_id: - try: - delete_dataset(integration_client, dataset_id) - except Exception: - pass + # Register for automatic cleanup + cleanup_datasets(dataset.dataset_id) + + # Verify dataset was created + assert dataset is not None + assert dataset.dataset_id is not None + assert dataset.name == dataset_config.dataset_name + + # Test all property accessors + assert dataset.data_type == "image" + assert hasattr(dataset, 'files_count') + assert hasattr(dataset, 'status_code') + assert hasattr(dataset, 'description') + assert hasattr(dataset, 'created_at') + assert hasattr(dataset, 'created_by') + + # Validate property types + assert isinstance(dataset.dataset_id, str) + assert isinstance(dataset.data_type, str) + assert isinstance(dataset.files_count, int) + assert isinstance(dataset.status_code, int) + if dataset.name is not None: + assert isinstance(dataset.name, str) + + # Print properties for verification + print(f"\nDataset Properties:") + print(f" ID: {dataset.dataset_id}") + print(f" Name: {dataset.name}") + print(f" Data Type: {dataset.data_type}") + print(f" Files Count: {dataset.files_count}") + print(f" Status Code: {dataset.status_code}") + print(f" Description: {dataset.description}") + + # Wait for dataset processing + status = dataset.status(timeout=300) # 5 min timeout + assert status is not None - @handle_api_errors def test_create_dataset_from_connection_with_existing_connection( self, integration_client, test_credentials, test_project_ids ): @@ -262,8 +305,7 @@ def test_create_dataset_from_connection_with_existing_connection( # Clean up delete_dataset(integration_client, dataset.dataset_id) - @handle_api_errors - def test_create_dataset_with_multimodal_indexing(self, integration_client): + def test_create_dataset_with_multimodal_indexing(self, integration_client, cleanup_datasets): """ Comprehensive test: multimodal indexing and dataset deletion. Tests dataset creation with multimodal indexing, then verifies deletion works correctly. @@ -283,39 +325,38 @@ def test_create_dataset_with_multimodal_indexing(self, integration_client): multimodal_indexing=True, ) - dataset_id = None + # Create dataset with multimodal indexing + dataset = create_dataset_from_local( + client=integration_client, + dataset_config=dataset_config, + folder_to_upload=tmpdir, + ) - try: - # Create dataset with multimodal indexing - dataset = create_dataset_from_local( - client=integration_client, - dataset_config=dataset_config, - folder_to_upload=tmpdir, - ) + assert dataset is not None + assert dataset.dataset_id is not None - assert dataset is not None - dataset_id = dataset.dataset_id - assert dataset_id is not None - # Register for cleanup - register_dataset_for_cleanup(dataset_id, integration_client) + # Register for automatic cleanup + cleanup_datasets(dataset.dataset_id) - # Verify multimodal indexing can be enabled + # Verify multimodal indexing can be enabled + try: result = dataset.enable_multimodal_indexing(is_multimodal=True) assert result is not None + except (LabellerrError, requests.exceptions.RetryError) as e: + pytest.fail(enhance_api_error( + e, + f"Failed to enable multimodal indexing for dataset {dataset.dataset_id}" + )) - # Test deletion - delete_result = delete_dataset(integration_client, dataset_id) + # Test deletion + try: + delete_result = delete_dataset(integration_client, dataset.dataset_id) assert delete_result is not None - - # Mark as deleted (don't verify by fetching as API may return 500 errors) - dataset_id = None - - finally: - if dataset_id: - try: - delete_dataset(integration_client, dataset_id) - except Exception: - pass + except (LabellerrError, requests.exceptions.RetryError) as e: + pytest.fail(enhance_api_error( + e, + f"Failed to delete dataset {dataset.dataset_id}" + )) @pytest.mark.integration @@ -336,27 +377,33 @@ def test_invalid_dataset_id_format_rejected(self, integration_client): LabellerrDataset(integration_client, invalid_id) def test_valid_uuid_format_but_nonexistent_dataset(self, integration_client): - """Test that valid UUID format but non-existent dataset returns proper error""" + """ + Test that valid UUID format but non-existent dataset returns proper error. + + Note: API may return 500 errors for certain nonexistent IDs (infrastructure issue), + which causes retry exhaustion. We accept either proper error response or retry error. + """ nonexistent_id = "00000000-0000-0000-0000-000000000000" try: - # This should fail because the dataset doesn't exist - with pytest.raises((InvalidDatasetError, LabellerrError)) as exc_info: - dataset = LabellerrDataset(integration_client, nonexistent_id) + with pytest.raises((InvalidDatasetError, LabellerrError, requests.exceptions.RetryError)) as exc_info: + LabellerrDataset(integration_client, nonexistent_id) + + # Verify it's not an auth error (credentials were validated upfront) + error_msg = str(exc_info.value).lower() - # Skip if we got auth error - if exc_info.value: - skip_if_auth_error(exc_info.value) + # Check for auth errors + if "403" in error_msg or "unauthorized" in error_msg: + pytest.fail("Got auth error instead of not found - credentials issue") - # Verify error message mentions dataset not found - assert "not found" in str(exc_info.value).lower() or "dataset" in str(exc_info.value).lower() + # Accept any error (404, 500, retry error) as valid for nonexistent dataset + assert any( + x in error_msg for x in ["not found", "dataset", "error", "500", "retry", "max retries"] + ), f"Expected error for nonexistent dataset, got: {exc_info.value}" except Exception as e: - # If we get RetryError or 500 errors, that's expected for non-existent datasets - if "RetryError" in str(type(e).__name__) or "500" in str(e): - pass # Expected - else: - raise + # Unexpected exception type + pytest.fail(f"Unexpected exception type: {type(e).__name__}: {e}") def test_empty_dataset_id_rejected(self, integration_client): """Test that empty dataset_id is rejected""" @@ -383,7 +430,6 @@ def test_delete_nonexistent_dataset(self, integration_client): class TestDatasetListingIntegration: """Integration tests for dataset listing""" - @handle_api_errors def test_list_datasets_client_scope(self, integration_client): """Test listing datasets with client scope""" datasets = list(list_datasets( @@ -402,7 +448,6 @@ def test_list_datasets_client_scope(self, integration_client): assert "dataset_id" in dataset # May have other fields like name, data_type, etc. - @handle_api_errors def test_list_datasets_auto_pagination(self, integration_client): """Test listing datasets with auto-pagination (page_size=-1)""" datasets = list(list_datasets( @@ -415,7 +460,6 @@ def test_list_datasets_auto_pagination(self, integration_client): # Should return a list assert isinstance(datasets, list) - @handle_api_errors def test_list_datasets_different_data_types(self, integration_client): """Test listing datasets for different data types""" data_types = ["image", "video", "document"] @@ -430,7 +474,6 @@ def test_list_datasets_different_data_types(self, integration_client): assert isinstance(datasets, list) - @handle_api_errors def test_list_datasets_project_scope(self, integration_client): """Test listing datasets with project scope""" datasets = list(list_datasets( @@ -448,79 +491,23 @@ def test_list_datasets_project_scope(self, integration_client): class TestDatasetWorkflowIntegration: """Integration tests for complete dataset workflows""" - @handle_api_errors - def test_dataset_update_operations(self, integration_client): + @pytest.mark.skip(reason="Update operations not yet implemented - placeholder for future feature") + def test_dataset_update_operations_not_implemented(self): """ - Test dataset update operations: name, description, and metadata. - - NOTE: This test currently documents that update operations are NOT YET IMPLEMENTED. - When update functionality is added to the SDK, this test will validate it. - For now, it verifies that datasets can be created and their properties accessed. - """ - # Get real test images - test_images = get_test_images_from_env(num_images=1) - - with tempfile.TemporaryDirectory() as tmpdir: - import shutil - - # Copy real image to temp directory - shutil.copy2(test_images[0], tmpdir) - - dataset_config = DatasetConfig( - dataset_name=f"Update Test Dataset {int(time.time())}", - dataset_description="Original description for update testing", - data_type="image", - ) - - dataset_id = None - - try: - # Create dataset - dataset = create_dataset_from_local( - client=integration_client, - dataset_config=dataset_config, - folder_to_upload=tmpdir, - ) + Placeholder test for dataset update operations. - assert dataset is not None - dataset_id = dataset.dataset_id - assert dataset_id is not None - # Register for cleanup - register_dataset_for_cleanup(dataset_id, integration_client) - - # Verify original properties are accessible - assert dataset.name == dataset_config.dataset_name - assert dataset.data_type == "image" - - # Document what update operations are NOT YET IMPLEMENTED: - print(f"\n⚠ Update operations not yet implemented in SDK:") - print(f" - update_name() - method does not exist") - print(f" - update_description() - method does not exist") - print(f" - update_metadata() - method does not exist") - print(f" - add_files() - method does not exist") - print(f" - remove_files() - method does not exist") - - # Verify that these methods don't exist (expected) - assert not hasattr(dataset, 'update_name'), "update_name unexpectedly exists" - assert not hasattr(dataset, 'update_description'), "update_description unexpectedly exists" - assert not hasattr(dataset, 'update_metadata'), "update_metadata unexpectedly exists" - assert not hasattr(dataset, 'add_files'), "add_files unexpectedly exists" - assert not hasattr(dataset, 'remove_files'), "remove_files unexpectedly exists" - - # Clean up - delete_result = delete_dataset(integration_client, dataset_id) - assert delete_result is not None - dataset_id = None + When update APIs are available, this test should verify: + - update_name() + - update_description() + - update_metadata() + - add_files() + - remove_files() - finally: - if dataset_id: - try: - delete_dataset(integration_client, dataset_id) - except Exception: - pass + TODO: Implement when update APIs are available + """ + pass - @handle_api_errors - def test_complete_dataset_lifecycle(self, integration_client): + def test_complete_dataset_lifecycle(self, integration_client, cleanup_datasets): """Test complete dataset lifecycle: create, fetch, list, and delete""" # Get real test images test_images = get_test_images_from_env(num_images=2) @@ -538,54 +525,68 @@ def test_complete_dataset_lifecycle(self, integration_client): data_type="image", ) - dataset_id = None + # Step 1: Create dataset + dataset = create_dataset_from_local( + client=integration_client, + dataset_config=dataset_config, + folder_to_upload=tmpdir, + ) - try: - # Step 1: Create dataset - dataset = create_dataset_from_local( - client=integration_client, - dataset_config=dataset_config, - folder_to_upload=tmpdir, - ) + assert dataset is not None + assert dataset.dataset_id is not None - assert dataset is not None - dataset_id = dataset.dataset_id - assert dataset_id is not None - # Register for cleanup - register_dataset_for_cleanup(dataset_id, integration_client) + # Register for automatic cleanup + cleanup_datasets(dataset.dataset_id) - # Step 2: Fetch dataset by ID - fetched_dataset = LabellerrDataset(integration_client, dataset_id) - assert fetched_dataset.dataset_id == dataset_id + # Step 2: Fetch dataset by ID + try: + fetched_dataset = LabellerrDataset(integration_client, dataset.dataset_id) + assert fetched_dataset.dataset_id == dataset.dataset_id assert fetched_dataset.name == dataset_config.dataset_name + except (LabellerrError, requests.exceptions.RetryError) as e: + pytest.fail(enhance_api_error( + e, + f"Step 2: Failed to fetch dataset by ID {dataset.dataset_id}" + )) - # Step 3: Check dataset status - status = fetched_dataset.status() + # Step 3: Check dataset status + try: + status = fetched_dataset.status(timeout=300) # 5 min timeout assert status is not None assert "status_code" in status + except (LabellerrError, requests.exceptions.RetryError) as e: + pytest.fail(enhance_api_error( + e, + f"Step 3: Failed to check status for dataset {dataset.dataset_id}" + )) - # Step 4: List datasets and verify our dataset is in the list - datasets = list(list_datasets( + # Step 4: List datasets and verify our dataset is in the list + try: + # Use pagination to find the dataset + found = False + for dataset_dict in list_datasets( client=integration_client, datatype="image", scope=DataSetScope.client, - page_size=20, + page_size=-1, # Auto-paginate to check all datasets + ): + if dataset_dict.get("dataset_id") == dataset.dataset_id: + found = True + break + + assert found, f"Created dataset {dataset.dataset_id} not found in listing" + except (LabellerrError, requests.exceptions.RetryError) as e: + pytest.fail(enhance_api_error( + e, + f"Step 4: Failed to list datasets or verify dataset {dataset.dataset_id} in listing" )) - dataset_ids = [d.get("dataset_id") for d in datasets] - # Our dataset might or might not be in the first page - # So we just verify the list operation worked - - # Step 5: Delete dataset - delete_result = delete_dataset(integration_client, dataset_id) + # Step 5: Delete dataset + try: + delete_result = delete_dataset(integration_client, dataset.dataset_id) assert delete_result is not None - - dataset_id = None # Mark as deleted - - finally: - # Cleanup: ensure dataset is deleted even if test fails - if dataset_id: - try: - delete_dataset(integration_client, dataset_id) - except Exception: - pass # Ignore cleanup errors + except (LabellerrError, requests.exceptions.RetryError) as e: + pytest.fail(enhance_api_error( + e, + f"Step 5: Failed to delete dataset {dataset.dataset_id}" + )) From 284f90cabec63c29dd73d238f5573d10771b65a4 Mon Sep 17 00:00:00 2001 From: nupursharma-labellerr Date: Fri, 9 Jan 2026 19:16:40 +0530 Subject: [PATCH 08/11] [LABIMP-8649]: Removing enhance_api_error() as it was overkill --- .../test_dataset_creation_integration.py | 151 ++++-------------- 1 file changed, 27 insertions(+), 124 deletions(-) diff --git a/tests/integration/test_dataset_creation_integration.py b/tests/integration/test_dataset_creation_integration.py index cbc23ff..b28f0d2 100644 --- a/tests/integration/test_dataset_creation_integration.py +++ b/tests/integration/test_dataset_creation_integration.py @@ -6,7 +6,6 @@ """ import os -import re import time import tempfile from pathlib import Path @@ -30,66 +29,6 @@ from labellerr.core.schemas import DatasetConfig, DataSetScope -def enhance_api_error(error: Exception, context: str) -> str: - """ - Enhance API error messages with context for better CI diagnostics. - - Args: - error: The original exception - context: Description of what operation was being performed - - Returns: - Enhanced error message with API details - """ - error_msg = str(error) - - # Check for HTML error responses (API returning error pages) - if "" in error_msg or " Date: Tue, 27 Jan 2026 10:32:17 +0530 Subject: [PATCH 09/11] Removed unnecessary unit test --- .../test_dataset_creation_integration.py | 4 +-- tests/unit/test_dataset_creation.py | 28 ------------------- 2 files changed, 2 insertions(+), 30 deletions(-) diff --git a/tests/integration/test_dataset_creation_integration.py b/tests/integration/test_dataset_creation_integration.py index b28f0d2..6d290f4 100644 --- a/tests/integration/test_dataset_creation_integration.py +++ b/tests/integration/test_dataset_creation_integration.py @@ -201,7 +201,7 @@ def test_create_dataset_from_local_folder(self, integration_client, cleanup_data assert isinstance(dataset.name, str) # Print properties for verification - print(f"\nDataset Properties:") + print("\nDataset Properties:") print(f" ID: {dataset.dataset_id}") print(f" Name: {dataset.name}") print(f" Data Type: {dataset.data_type}") @@ -214,7 +214,7 @@ def test_create_dataset_from_local_folder(self, integration_client, cleanup_data assert status is not None def test_create_dataset_from_connection_with_existing_connection( - self, integration_client, test_credentials, test_project_ids + self, integration_client ): """Test creating a dataset using an existing connection""" # Skip if no connection ID available diff --git a/tests/unit/test_dataset_creation.py b/tests/unit/test_dataset_creation.py index 38cc2da..f68b9ea 100644 --- a/tests/unit/test_dataset_creation.py +++ b/tests/unit/test_dataset_creation.py @@ -325,34 +325,6 @@ def test_list_datasets_with_last_dataset_id(self, client): class TestDatasetValidation: """Test dataset ID validation""" - def test_valid_uuid_format(self, client): - """Test that valid UUID formats pass validation""" - valid_uuids = [ - "550e8400-e29b-41d4-a716-446655440000", - "1c8b2a05-0321-44fd-91e3-2ea911382cf9", - "00000000-0000-0000-0000-000000000000", - ] - - for dataset_id in valid_uuids: - # Validation should pass, but API call will fail (mocked) - with patch.object(client, "make_request", side_effect=LabellerrError("API error")): - with pytest.raises(LabellerrError): - LabellerrDatasetMeta.get_dataset(client, dataset_id) - - def test_invalid_uuid_format_rejected(self, client): - """Test that invalid UUID formats are rejected before API call""" - invalid_ids = [ - "invalid-id", - "not-a-uuid", - "123456", - "05becc9c-e221-42ea-90f8-8d24031e2f3b1", # Extra character - "05becc9c-e221-42ea-90f8", # Too short - ] - - for dataset_id in invalid_ids: - with pytest.raises(InvalidDatasetIDError, match="Invalid dataset ID format"): - LabellerrDatasetMeta.get_dataset(client, dataset_id) - def test_empty_dataset_id(self, client): """Test that empty dataset_id is rejected""" with pytest.raises(InvalidDatasetIDError, match="Dataset ID cannot be None or empty"): From 16fe7448a3c49c7fdd7e85a9c4bfa2c9f1e601e5 Mon Sep 17 00:00:00 2001 From: Ximi Hoque Date: Tue, 27 Jan 2026 11:51:19 +0530 Subject: [PATCH 10/11] Updated CI --- .github/workflows/ci.yml | 8 ++++++++ .github/workflows/claude-code-review.yml | 10 ++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ff18c93..804daa9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,7 @@ on: jobs: test: runs-on: ubuntu-latest + environment: prod env: API_KEY: ${{ secrets.API_KEY }} @@ -20,6 +21,13 @@ jobs: GCS_CONNECTION_IMAGE: ${{ secrets.GCS_CONNECTION_IMAGE }} GCS_CONNECTION_VIDEO: ${{ secrets.GCS_CONNECTION_VIDEO }} LABELLERR_TEST_DATA_PATH: ${{ github.workspace }}/tests/fixtures/mcp_images + IMAGE_DATASET_ID: ${{ vars.IMAGE_DATASET_ID }} + AUDIO_MP3_DATASET_ID: ${{ vars.AUDIO_MP3_DATASET_ID }} + AUDIO_WAV_DATASET_ID: ${{ vars.AUDIO_WAV_DATASET_ID }} + VIDEO_DATASET_ID: ${{ vars.VIDEO_DATASET_ID }} + DOCUMENT_DATASET_ID: ${{ vars.DOCUMENT_DATASET_ID }} + TEXT_DATASET_ID: ${{ vars.TEXT_DATASET_ID }} + steps: - name: Checkout diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 8fc1ed4..cf20b6c 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -1,14 +1,20 @@ name: Claude Auto Review on: pull_request: - types: [opened, synchronize] + types: [opened] paths-ignore: - "**/*.md" - "docs/**" + issue_comment: + types: [created] jobs: review: - if: github.event.pull_request.update_count < 3 + if: | + (github.event_name == 'pull_request' && github.event.action == 'opened') || + (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(github.event.comment.body, '@claude')) runs-on: ubuntu-latest permissions: contents: read From 428bb857ee9b4cce02c7115ec0cf6a0b1e51399d Mon Sep 17 00:00:00 2001 From: Ximi Hoque Date: Tue, 27 Jan 2026 11:54:22 +0530 Subject: [PATCH 11/11] Formatting --- .../test_dataset_creation_integration.py | 115 +++++++++------ tests/unit/test_dataset_creation.py | 135 +++++++++++------- 2 files changed, 160 insertions(+), 90 deletions(-) diff --git a/tests/integration/test_dataset_creation_integration.py b/tests/integration/test_dataset_creation_integration.py index 6d290f4..b3a019b 100644 --- a/tests/integration/test_dataset_creation_integration.py +++ b/tests/integration/test_dataset_creation_integration.py @@ -53,6 +53,7 @@ def _register(dataset_id: str): delete_dataset(integration_client, dataset_id) except Exception as e: import logging + logging.warning(f"Failed to cleanup dataset {dataset_id}: {e}") @@ -119,9 +120,10 @@ def get_test_images_from_env(num_images: int = 3) -> list: pytest.skip(f"IMG_DATASET_PATH is not a directory: {img_path}") # Find image files (jpg, jpeg, png) - image_extensions = {'.jpg', '.jpeg', '.png', '.gif', '.bmp'} + image_extensions = {".jpg", ".jpeg", ".png", ".gif", ".bmp"} image_files = [ - f for f in img_path.iterdir() + f + for f in img_path.iterdir() if f.is_file() and f.suffix.lower() in image_extensions ] @@ -129,7 +131,9 @@ def get_test_images_from_env(num_images: int = 3) -> list: pytest.skip(f"No image files found in IMG_DATASET_PATH: {img_path}") if len(image_files) < num_images: - pytest.skip(f"Not enough images in IMG_DATASET_PATH. Found {len(image_files)}, need {num_images}") + pytest.skip( + f"Not enough images in IMG_DATASET_PATH. Found {len(image_files)}, need {num_images}" + ) # Return first num_images files return image_files[:num_images] @@ -139,7 +143,9 @@ def get_test_images_from_env(num_images: int = 3) -> list: class TestDatasetCreationIntegration: """Integration tests for dataset creation""" - def test_create_dataset_from_local_folder(self, integration_client, cleanup_datasets): + def test_create_dataset_from_local_folder( + self, integration_client, cleanup_datasets + ): """ Comprehensive test: dataset creation, all properties validation, and property types. Tests: @@ -161,7 +167,9 @@ def test_create_dataset_from_local_folder(self, integration_client, cleanup_data # Verify files were copied copied_files = list(Path(tmpdir).iterdir()) - assert len(copied_files) == 3, f"Expected 3 files, found {len(copied_files)}" + assert ( + len(copied_files) == 3 + ), f"Expected 3 files, found {len(copied_files)}" dataset_config = DatasetConfig( dataset_name=f"Test Local Dataset {int(time.time())}", @@ -186,11 +194,11 @@ def test_create_dataset_from_local_folder(self, integration_client, cleanup_data # Test all property accessors assert dataset.data_type == "image" - assert hasattr(dataset, 'files_count') - assert hasattr(dataset, 'status_code') - assert hasattr(dataset, 'description') - assert hasattr(dataset, 'created_at') - assert hasattr(dataset, 'created_by') + assert hasattr(dataset, "files_count") + assert hasattr(dataset, "status_code") + assert hasattr(dataset, "description") + assert hasattr(dataset, "created_at") + assert hasattr(dataset, "created_by") # Validate property types assert isinstance(dataset.dataset_id, str) @@ -244,7 +252,9 @@ def test_create_dataset_from_connection_with_existing_connection( # Clean up delete_dataset(integration_client, dataset.dataset_id) - def test_create_dataset_with_multimodal_indexing(self, integration_client, cleanup_datasets): + def test_create_dataset_with_multimodal_indexing( + self, integration_client, cleanup_datasets + ): """ Comprehensive test: multimodal indexing and dataset deletion. Tests dataset creation with multimodal indexing, then verifies deletion works correctly. @@ -300,7 +310,9 @@ def test_invalid_dataset_id_format_rejected(self, integration_client): ] for invalid_id in invalid_ids: - with pytest.raises(InvalidDatasetIDError, match="Invalid dataset ID format"): + with pytest.raises( + InvalidDatasetIDError, match="Invalid dataset ID format" + ): LabellerrDataset(integration_client, invalid_id) def test_valid_uuid_format_but_nonexistent_dataset(self, integration_client): @@ -313,7 +325,9 @@ def test_valid_uuid_format_but_nonexistent_dataset(self, integration_client): nonexistent_id = "00000000-0000-0000-0000-000000000000" try: - with pytest.raises((InvalidDatasetError, LabellerrError, requests.exceptions.RetryError)) as exc_info: + with pytest.raises( + (InvalidDatasetError, LabellerrError, requests.exceptions.RetryError) + ) as exc_info: LabellerrDataset(integration_client, nonexistent_id) # Verify it's not an auth error (credentials were validated upfront) @@ -325,7 +339,15 @@ def test_valid_uuid_format_but_nonexistent_dataset(self, integration_client): # Accept any error (404, 500, retry error) as valid for nonexistent dataset assert any( - x in error_msg for x in ["not found", "dataset", "error", "500", "retry", "max retries"] + x in error_msg + for x in [ + "not found", + "dataset", + "error", + "500", + "retry", + "max retries", + ] ), f"Expected error for nonexistent dataset, got: {exc_info.value}" except Exception as e: @@ -334,9 +356,12 @@ def test_valid_uuid_format_but_nonexistent_dataset(self, integration_client): def test_empty_dataset_id_rejected(self, integration_client): """Test that empty dataset_id is rejected""" - with pytest.raises(InvalidDatasetIDError, match="Dataset ID cannot be None or empty"): + with pytest.raises( + InvalidDatasetIDError, match="Dataset ID cannot be None or empty" + ): LabellerrDataset(integration_client, "") + @pytest.mark.integration class TestDatasetDeletionIntegration: """Integration tests for dataset deletion""" @@ -359,12 +384,14 @@ class TestDatasetListingIntegration: def test_list_datasets_client_scope(self, integration_client): """Test listing datasets with client scope""" - datasets = list(list_datasets( - client=integration_client, - datatype="image", - scope=DataSetScope.client, - page_size=10, - )) + datasets = list( + list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.client, + page_size=10, + ) + ) # Should return a list (may be empty) assert isinstance(datasets, list) @@ -377,12 +404,14 @@ def test_list_datasets_client_scope(self, integration_client): def test_list_datasets_auto_pagination(self, integration_client): """Test listing datasets with auto-pagination (page_size=-1)""" - datasets = list(list_datasets( - client=integration_client, - datatype="image", - scope=DataSetScope.client, - page_size=-1, # Auto-pagination - )) + datasets = list( + list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.client, + page_size=-1, # Auto-pagination + ) + ) # Should return a list assert isinstance(datasets, list) @@ -392,23 +421,27 @@ def test_list_datasets_different_data_types(self, integration_client): data_types = ["image", "video", "document"] for data_type in data_types: - datasets = list(list_datasets( - client=integration_client, - datatype=data_type, - scope=DataSetScope.client, - page_size=5, - )) + datasets = list( + list_datasets( + client=integration_client, + datatype=data_type, + scope=DataSetScope.client, + page_size=5, + ) + ) assert isinstance(datasets, list) def test_list_datasets_project_scope(self, integration_client): """Test listing datasets with project scope""" - datasets = list(list_datasets( - client=integration_client, - datatype="image", - scope=DataSetScope.project, - page_size=10, - )) + datasets = list( + list_datasets( + client=integration_client, + datatype="image", + scope=DataSetScope.project, + page_size=10, + ) + ) # Should return a list (may be empty) assert isinstance(datasets, list) @@ -418,7 +451,9 @@ def test_list_datasets_project_scope(self, integration_client): class TestDatasetWorkflowIntegration: """Integration tests for complete dataset workflows""" - @pytest.mark.skip(reason="Update operations not yet implemented - placeholder for future feature") + @pytest.mark.skip( + reason="Update operations not yet implemented - placeholder for future feature" + ) def test_dataset_update_operations_not_implemented(self): """ Placeholder test for dataset update operations. diff --git a/tests/unit/test_dataset_creation.py b/tests/unit/test_dataset_creation.py index f68b9ea..2dd3a3b 100644 --- a/tests/unit/test_dataset_creation.py +++ b/tests/unit/test_dataset_creation.py @@ -5,7 +5,7 @@ including validation, error handling, and edge cases. """ -from unittest.mock import Mock, patch, MagicMock +from unittest.mock import Mock, patch import pytest from labellerr.core.datasets import ( @@ -15,11 +15,7 @@ list_datasets, ) from labellerr.core.datasets.base import LabellerrDataset, LabellerrDatasetMeta -from labellerr.core.exceptions import ( - LabellerrError, - InvalidDatasetIDError, - InvalidDatasetError, -) +from labellerr.core.exceptions import LabellerrError, InvalidDatasetIDError from labellerr.core.schemas import DatasetConfig, DataSetScope @@ -35,13 +31,19 @@ def test_create_dataset_from_connection_with_string_connection_id(self, client): ) mock_response = { - "response": {"dataset_id": "550e8400-e29b-41d4-a716-446655440000", "data_type": "image"} + "response": { + "dataset_id": "550e8400-e29b-41d4-a716-446655440000", + "data_type": "image", + } } with patch.object(client, "make_request", return_value=mock_response): with patch( "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", - return_value={"dataset_id": "550e8400-e29b-41d4-a716-446655440000", "data_type": "image"}, + return_value={ + "dataset_id": "550e8400-e29b-41d4-a716-446655440000", + "data_type": "image", + }, ): dataset = create_dataset_from_connection( client=client, @@ -67,13 +69,19 @@ def test_create_dataset_from_connection_with_connection_object(self, client): mock_connection.connection_id = "test-connection-id" mock_response = { - "response": {"dataset_id": "550e8400-e29b-41d4-a716-446655440001", "data_type": "video"} + "response": { + "dataset_id": "550e8400-e29b-41d4-a716-446655440001", + "data_type": "video", + } } with patch.object(client, "make_request", return_value=mock_response): with patch( "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", - return_value={"dataset_id": "550e8400-e29b-41d4-a716-446655440001", "data_type": "video"}, + return_value={ + "dataset_id": "550e8400-e29b-41d4-a716-446655440001", + "data_type": "video", + }, ): dataset = create_dataset_from_connection( client=client, @@ -93,8 +101,12 @@ def test_create_dataset_from_local_with_files_list(self, client): files_to_upload = ["/path/to/file1.jpg", "/path/to/file2.jpg"] - with patch("labellerr.core.datasets.upload_files", return_value="local-connection-id"): - with patch("labellerr.core.datasets.create_dataset_from_connection") as mock_create: + with patch( + "labellerr.core.datasets.upload_files", return_value="local-connection-id" + ): + with patch( + "labellerr.core.datasets.create_dataset_from_connection" + ) as mock_create: mock_dataset = Mock() mock_dataset.dataset_id = "test-dataset-id" mock_create.return_value = mock_dataset @@ -119,9 +131,11 @@ def test_create_dataset_from_local_with_folder(self, client): with patch( "labellerr.core.datasets.upload_folder_files_to_dataset", - return_value={"connection_id": "folder-connection-id", "status": "success"} + return_value={"connection_id": "folder-connection-id", "status": "success"}, ): - with patch("labellerr.core.datasets.create_dataset_from_connection") as mock_create: + with patch( + "labellerr.core.datasets.create_dataset_from_connection" + ) as mock_create: mock_dataset = Mock() mock_dataset.dataset_id = "test-dataset-id" mock_create.return_value = mock_dataset @@ -141,7 +155,9 @@ def test_create_dataset_from_local_no_source(self, client): data_type="image", ) - with pytest.raises(LabellerrError, match="No files or folder to upload provided"): + with pytest.raises( + LabellerrError, match="No files or folder to upload provided" + ): create_dataset_from_local( client=client, dataset_config=dataset_config, @@ -159,12 +175,14 @@ def test_create_dataset_with_multimodal_indexing(self, client): "response": {"dataset_id": "test-dataset-id", "data_type": "image"} } - with patch.object(client, "make_request", return_value=mock_response) as mock_request: + with patch.object( + client, "make_request", return_value=mock_response + ) as mock_request: with patch( "labellerr.core.datasets.base.LabellerrDatasetMeta.get_dataset", return_value={"dataset_id": "test-dataset-id", "data_type": "image"}, ): - dataset = create_dataset_from_connection( + create_dataset_from_connection( client=client, dataset_config=dataset_config, connection="test-connection", @@ -175,6 +193,7 @@ def test_create_dataset_with_multimodal_indexing(self, client): call_args = mock_request.call_args assert "data" in call_args.kwargs import json + payload = json.loads(call_args.kwargs["data"]) assert payload["es_multimodal_index"] is True @@ -187,9 +206,7 @@ def test_delete_dataset_success(self, client): """Test successful dataset deletion""" dataset_id = "550e8400-e29b-41d4-a716-446655440000" - mock_response = { - "response": {"status": "deleted", "dataset_id": dataset_id} - } + mock_response = {"response": {"status": "deleted", "dataset_id": dataset_id}} with patch.object(client, "make_request", return_value=mock_response): result = delete_dataset(client, dataset_id) @@ -203,7 +220,9 @@ def test_delete_dataset_invalid_id(self, client): # The deletion function doesn't validate UUID format before making request # So it will make the API call which should fail - with patch.object(client, "make_request", side_effect=LabellerrError("Invalid dataset ID")): + with patch.object( + client, "make_request", side_effect=LabellerrError("Invalid dataset ID") + ): with pytest.raises(LabellerrError): delete_dataset(client, invalid_id) @@ -211,7 +230,9 @@ def test_delete_nonexistent_dataset(self, client): """Test deletion of non-existent dataset""" dataset_id = "00000000-0000-0000-0000-000000000000" - with patch.object(client, "make_request", side_effect=LabellerrError("Dataset not found")): + with patch.object( + client, "make_request", side_effect=LabellerrError("Dataset not found") + ): with pytest.raises(LabellerrError, match="Dataset not found"): delete_dataset(client, dataset_id) @@ -233,12 +254,14 @@ def test_list_datasets_single_page(self, client): } with patch.object(client, "make_request", return_value=mock_response): - datasets = list(list_datasets( - client=client, - datatype="image", - scope=DataSetScope.client, - page_size=10, - )) + datasets = list( + list_datasets( + client=client, + datatype="image", + scope=DataSetScope.client, + page_size=10, + ) + ) assert len(datasets) == 2 assert datasets[0]["dataset_id"] == "id1" @@ -263,12 +286,14 @@ def test_list_datasets_auto_pagination(self, client): ] with patch.object(client, "make_request", side_effect=mock_responses): - datasets = list(list_datasets( - client=client, - datatype="image", - scope=DataSetScope.client, - page_size=-1, # Auto-pagination - )) + datasets = list( + list_datasets( + client=client, + datatype="image", + scope=DataSetScope.client, + page_size=-1, # Auto-pagination + ) + ) assert len(datasets) == 15 assert datasets[0]["dataset_id"] == "id0" @@ -284,12 +309,14 @@ def test_list_datasets_empty_result(self, client): } with patch.object(client, "make_request", return_value=mock_response): - datasets = list(list_datasets( - client=client, - datatype="video", - scope="user", # Use string instead of enum - page_size=10, - )) + datasets = list( + list_datasets( + client=client, + datatype="video", + scope="user", # Use string instead of enum + page_size=10, + ) + ) assert len(datasets) == 0 @@ -306,19 +333,25 @@ def test_list_datasets_with_last_dataset_id(self, client): } } - with patch.object(client, "make_request", return_value=mock_response) as mock_request: - datasets = list(list_datasets( - client=client, - datatype="document", - scope=DataSetScope.client, - page_size=10, - last_dataset_id="id10", - )) + with patch.object( + client, "make_request", return_value=mock_response + ) as mock_request: + datasets = list( + list_datasets( + client=client, + datatype="document", + scope=DataSetScope.client, + page_size=10, + last_dataset_id="id10", + ) + ) assert len(datasets) == 2 # Verify last_dataset_id was included in URL call_args = mock_request.call_args - assert "last_dataset_id=id10" in call_args[0][1] # URL is second positional arg + assert ( + "last_dataset_id=id10" in call_args[0][1] + ) # URL is second positional arg @pytest.mark.unit @@ -327,7 +360,9 @@ class TestDatasetValidation: def test_empty_dataset_id(self, client): """Test that empty dataset_id is rejected""" - with pytest.raises(InvalidDatasetIDError, match="Dataset ID cannot be None or empty"): + with pytest.raises( + InvalidDatasetIDError, match="Dataset ID cannot be None or empty" + ): LabellerrDatasetMeta.get_dataset(client, "") def test_none_dataset_id(self, client):