From ebec9a720b661ecbacc7f11af07dd8bd85eaba4a Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Thu, 5 Mar 2026 02:04:34 -0600 Subject: [PATCH] test: implement test_validate_transaction_exception_handling (#35) Removes the @pytest.mark.skip and implements the test using a patched RosterValidation that raises on the first call, triggering the except clause in validate_transaction. Verifies the method returns is_legal=False with a descriptive error message. Co-Authored-By: Claude Sonnet 4.6 --- tests/test_services_transaction.py | 525 +++++++++++++++++------------ 1 file changed, 313 insertions(+), 212 deletions(-) diff --git a/tests/test_services_transaction.py b/tests/test_services_transaction.py index 9c87e53..781d3f6 100644 --- a/tests/test_services_transaction.py +++ b/tests/test_services_transaction.py @@ -3,6 +3,7 @@ Tests for TransactionService Validates transaction service functionality, API interaction, and business logic. """ + import pytest from unittest.mock import AsyncMock, patch @@ -13,117 +14,131 @@ from exceptions import APIException class TestTransactionService: """Test TransactionService functionality.""" - + @pytest.fixture def service(self): """Create a fresh TransactionService instance for testing.""" return TransactionService() - + @pytest.fixture def mock_transaction_data(self): """Create mock transaction data for testing.""" return { - 'id': 27787, - 'week': 10, - 'season': 12, - 'moveid': 'Season-012-Week-10-19-13:04:41', - 'player': { - 'id': 12472, - 'name': 'Test Player', - 'wara': 2.47, - 'season': 12, - 'pos_1': 'LF' + "id": 27787, + "week": 10, + "season": 12, + "moveid": "Season-012-Week-10-19-13:04:41", + "player": { + "id": 12472, + "name": "Test Player", + "wara": 2.47, + "season": 12, + "pos_1": "LF", }, - 'oldteam': { - 'id': 508, - 'abbrev': 'NYD', - 'sname': 'Diamonds', - 'lname': 'New York Diamonds', - 'season': 12 + "oldteam": { + "id": 508, + "abbrev": "NYD", + "sname": "Diamonds", + "lname": "New York Diamonds", + "season": 12, }, - 'newteam': { - 'id': 499, - 'abbrev': 'WV', - 'sname': 'Black Bears', - 'lname': 'West Virginia Black Bears', - 'season': 12 + "newteam": { + "id": 499, + "abbrev": "WV", + "sname": "Black Bears", + "lname": "West Virginia Black Bears", + "season": 12, }, - 'cancelled': False, - 'frozen': False + "cancelled": False, + "frozen": False, } - - @pytest.fixture + + @pytest.fixture def mock_api_response(self, mock_transaction_data): """Create mock API response with multiple transactions.""" return { - 'count': 3, - 'transactions': [ + "count": 3, + "transactions": [ mock_transaction_data, - {**mock_transaction_data, 'id': 27788, 'frozen': True}, - {**mock_transaction_data, 'id': 27789, 'cancelled': True} - ] + {**mock_transaction_data, "id": 27788, "frozen": True}, + {**mock_transaction_data, "id": 27789, "cancelled": True}, + ], } - + @pytest.mark.asyncio async def test_service_initialization(self, service): """Test service initialization.""" assert service.model_class == Transaction - assert service.endpoint == 'transactions' - + assert service.endpoint == "transactions" + @pytest.mark.asyncio async def test_get_team_transactions_basic(self, service, mock_api_response): """Test getting team transactions with basic parameters.""" - with patch.object(service, 'get_all_items', new_callable=AsyncMock) as mock_get: + with patch.object(service, "get_all_items", new_callable=AsyncMock) as mock_get: mock_get.return_value = [ - Transaction.from_api_data(tx) for tx in mock_api_response['transactions'] + Transaction.from_api_data(tx) + for tx in mock_api_response["transactions"] ] - - result = await service.get_team_transactions('WV', 12) - + + result = await service.get_team_transactions("WV", 12) + assert len(result) == 3 assert all(isinstance(tx, Transaction) for tx in result) - + # Verify API call was made mock_get.assert_called_once() - + @pytest.mark.asyncio async def test_get_team_transactions_with_filters(self, service, mock_api_response): """Test getting team transactions with status filters.""" - with patch.object(service, 'get_all_items', new_callable=AsyncMock) as mock_get: + with patch.object(service, "get_all_items", new_callable=AsyncMock) as mock_get: mock_get.return_value = [] - + await service.get_team_transactions( - 'WV', 12, - cancelled=True, - frozen=False, - week_start=5, - week_end=15 + "WV", 12, cancelled=True, frozen=False, week_start=5, week_end=15 ) - + # Verify API call was made mock_get.assert_called_once() - + @pytest.mark.asyncio async def test_get_team_transactions_sorting(self, service, mock_transaction_data): """Test transaction sorting by week and moveid.""" # Create transactions with different weeks and moveids transactions_data = [ - {**mock_transaction_data, 'id': 1, 'week': 10, 'moveid': 'Season-012-Week-10-19-13:04:41'}, - {**mock_transaction_data, 'id': 2, 'week': 8, 'moveid': 'Season-012-Week-08-12-10:30:15'}, - {**mock_transaction_data, 'id': 3, 'week': 10, 'moveid': 'Season-012-Week-10-15-09:22:33'}, + { + **mock_transaction_data, + "id": 1, + "week": 10, + "moveid": "Season-012-Week-10-19-13:04:41", + }, + { + **mock_transaction_data, + "id": 2, + "week": 8, + "moveid": "Season-012-Week-08-12-10:30:15", + }, + { + **mock_transaction_data, + "id": 3, + "week": 10, + "moveid": "Season-012-Week-10-15-09:22:33", + }, ] - - with patch.object(service, 'get_all_items', new_callable=AsyncMock) as mock_get: - mock_get.return_value = [Transaction.from_api_data(tx) for tx in transactions_data] - - result = await service.get_team_transactions('WV', 12) - + + with patch.object(service, "get_all_items", new_callable=AsyncMock) as mock_get: + mock_get.return_value = [ + Transaction.from_api_data(tx) for tx in transactions_data + ] + + result = await service.get_team_transactions("WV", 12) + # Verify sorting: week 8 first, then week 10 sorted by moveid assert result[0].week == 8 assert result[1].week == 10 assert result[2].week == 10 assert result[1].moveid < result[2].moveid # Alphabetical order - + @pytest.mark.asyncio async def test_get_pending_transactions(self, service): """Test getting pending transactions. @@ -131,115 +146,166 @@ class TestTransactionService: The method first fetches the current week, then calls get_team_transactions with week_start set to the current week. """ - with patch.object(service, 'get_client', new_callable=AsyncMock) as mock_get_client: + with patch.object( + service, "get_client", new_callable=AsyncMock + ) as mock_get_client: mock_client = AsyncMock() - mock_client.get.return_value = {'week': 17} # Simulate current week + mock_client.get.return_value = {"week": 17} # Simulate current week mock_get_client.return_value = mock_client - with patch.object(service, 'get_team_transactions', new_callable=AsyncMock) as mock_get: + with patch.object( + service, "get_team_transactions", new_callable=AsyncMock + ) as mock_get: mock_get.return_value = [] - await service.get_pending_transactions('WV', 12) + await service.get_pending_transactions("WV", 12) + + mock_get.assert_called_once_with( + "WV", 12, cancelled=False, frozen=False, week_start=17 + ) - mock_get.assert_called_once_with('WV', 12, cancelled=False, frozen=False, week_start=17) - @pytest.mark.asyncio async def test_get_frozen_transactions(self, service): - """Test getting frozen transactions.""" - with patch.object(service, 'get_team_transactions', new_callable=AsyncMock) as mock_get: + """Test getting frozen transactions.""" + with patch.object( + service, "get_team_transactions", new_callable=AsyncMock + ) as mock_get: mock_get.return_value = [] - - await service.get_frozen_transactions('WV', 12) - - mock_get.assert_called_once_with('WV', 12, frozen=True) - + + await service.get_frozen_transactions("WV", 12) + + mock_get.assert_called_once_with("WV", 12, frozen=True) + @pytest.mark.asyncio - async def test_get_processed_transactions_success(self, service, mock_transaction_data): + async def test_get_processed_transactions_success( + self, service, mock_transaction_data + ): """Test getting processed transactions with current week lookup.""" # Mock current week response - current_response = {'week': 12} - + current_response = {"week": 12} + # Create test transactions with different statuses all_transactions = [ - Transaction.from_api_data({**mock_transaction_data, 'id': 1, 'cancelled': False, 'frozen': False}), # pending - Transaction.from_api_data({**mock_transaction_data, 'id': 2, 'cancelled': False, 'frozen': True}), # frozen - Transaction.from_api_data({**mock_transaction_data, 'id': 3, 'cancelled': True, 'frozen': False}), # cancelled - Transaction.from_api_data({**mock_transaction_data, 'id': 4, 'cancelled': False, 'frozen': False}), # pending + Transaction.from_api_data( + {**mock_transaction_data, "id": 1, "cancelled": False, "frozen": False} + ), # pending + Transaction.from_api_data( + {**mock_transaction_data, "id": 2, "cancelled": False, "frozen": True} + ), # frozen + Transaction.from_api_data( + {**mock_transaction_data, "id": 3, "cancelled": True, "frozen": False} + ), # cancelled + Transaction.from_api_data( + {**mock_transaction_data, "id": 4, "cancelled": False, "frozen": False} + ), # pending ] - + # Mock the service methods - with patch.object(service, 'get_client', new_callable=AsyncMock) as mock_client: + with patch.object(service, "get_client", new_callable=AsyncMock) as mock_client: mock_api_client = AsyncMock() mock_api_client.get.return_value = current_response mock_client.return_value = mock_api_client - - with patch.object(service, 'get_team_transactions', new_callable=AsyncMock) as mock_get_team: + + with patch.object( + service, "get_team_transactions", new_callable=AsyncMock + ) as mock_get_team: mock_get_team.return_value = all_transactions - - result = await service.get_processed_transactions('WV', 12) - + + result = await service.get_processed_transactions("WV", 12) + # Should return empty list since all test transactions are either pending, frozen, or cancelled # (none are processed - not pending, not frozen, not cancelled) assert len(result) == 0 - + # Verify current week API call - mock_api_client.get.assert_called_once_with('current') - + mock_api_client.get.assert_called_once_with("current") + # Verify team transactions call with week range - mock_get_team.assert_called_once_with('WV', 12, week_start=8) # 12 - 4 = 8 - + mock_get_team.assert_called_once_with( + "WV", 12, week_start=8 + ) # 12 - 4 = 8 + @pytest.mark.asyncio async def test_get_processed_transactions_fallback(self, service): """Test processed transactions fallback when current week fails.""" - with patch.object(service, 'get_client', new_callable=AsyncMock) as mock_client: + with patch.object(service, "get_client", new_callable=AsyncMock) as mock_client: # Mock client to raise exception mock_client.side_effect = Exception("API Error") - - with patch.object(service, 'get_team_transactions', new_callable=AsyncMock) as mock_get_team: + + with patch.object( + service, "get_team_transactions", new_callable=AsyncMock + ) as mock_get_team: mock_get_team.return_value = [] - - result = await service.get_processed_transactions('WV', 12) - + + result = await service.get_processed_transactions("WV", 12) + assert result == [] # Verify fallback call without week range - mock_get_team.assert_called_with('WV', 12) - + mock_get_team.assert_called_with("WV", 12) + @pytest.mark.asyncio async def test_validate_transaction_success(self, service, mock_transaction_data): """Test successful transaction validation.""" transaction = Transaction.from_api_data(mock_transaction_data) - + result = await service.validate_transaction(transaction) - + assert isinstance(result, RosterValidation) assert result.is_legal is True assert len(result.errors) == 0 - + @pytest.mark.asyncio async def test_validate_transaction_no_moves(self, service, mock_transaction_data): """Test transaction validation with no moves (edge case).""" # For single-move transactions, this test simulates validation logic transaction = Transaction.from_api_data(mock_transaction_data) - + # Mock validation that would fail for complex business rules - with patch.object(service, 'validate_transaction') as mock_validate: + with patch.object(service, "validate_transaction") as mock_validate: validation_result = RosterValidation( - is_legal=False, - errors=['Transaction validation failed'] + is_legal=False, errors=["Transaction validation failed"] ) mock_validate.return_value = validation_result - + result = await service.validate_transaction(transaction) - + assert result.is_legal is False - assert 'Transaction validation failed' in result.errors - - @pytest.mark.skip(reason="Exception handling test needs refactoring for new patterns") + assert "Transaction validation failed" in result.errors + @pytest.mark.asyncio - async def test_validate_transaction_exception_handling(self, service, mock_transaction_data): - """Test transaction validation exception handling.""" - pass - + async def test_validate_transaction_exception_handling( + self, service, mock_transaction_data + ): + """Test transaction validation exception handling. + + When an unexpected exception occurs inside validate_transaction (e.g., the + RosterValidation constructor raises), the method's except clause catches it + and returns a failed RosterValidation containing the error message rather + than propagating the exception to the caller. + + Covers the critical except path at services/transaction_service.py:187-192. + """ + transaction = Transaction.from_api_data(mock_transaction_data) + + _real = RosterValidation + call_count = [0] + + def patched_rv(*args, **kwargs): + call_count[0] += 1 + if call_count[0] == 1: + raise RuntimeError("Simulated validation failure") + return _real(*args, **kwargs) + + with patch( + "services.transaction_service.RosterValidation", side_effect=patched_rv + ): + result = await service.validate_transaction(transaction) + + assert isinstance(result, RosterValidation) + assert result.is_legal is False + assert len(result.errors) == 1 + assert result.errors[0] == "Validation error: Simulated validation failure" + @pytest.mark.asyncio async def test_cancel_transaction_success(self, service, mock_transaction_data): """Test successful transaction cancellation. @@ -248,51 +314,57 @@ class TestTransactionService: returning a success message for bulk updates. We mock the client.patch() method to simulate successful cancellation. """ - with patch.object(service, 'get_client', new_callable=AsyncMock) as mock_get_client: + with patch.object( + service, "get_client", new_callable=AsyncMock + ) as mock_get_client: mock_client = AsyncMock() # cancel_transaction expects a string response for success mock_client.patch.return_value = "Updated 1 transactions" mock_get_client.return_value = mock_client - result = await service.cancel_transaction('27787') + result = await service.cancel_transaction("27787") assert result is True mock_client.patch.assert_called_once() call_args = mock_client.patch.call_args - assert call_args[0][0] == 'transactions' # endpoint - assert 'cancelled' in call_args[0][1] # update_data contains 'cancelled' - assert call_args[1]['object_id'] == '27787' # transaction_id - + assert call_args[0][0] == "transactions" # endpoint + assert "cancelled" in call_args[0][1] # update_data contains 'cancelled' + assert call_args[1]["object_id"] == "27787" # transaction_id + @pytest.mark.asyncio async def test_cancel_transaction_not_found(self, service): """Test cancelling non-existent transaction. When the API returns None (no response), cancel_transaction returns False. """ - with patch.object(service, 'get_client', new_callable=AsyncMock) as mock_get_client: + with patch.object( + service, "get_client", new_callable=AsyncMock + ) as mock_get_client: mock_client = AsyncMock() mock_client.patch.return_value = None # No response = failure mock_get_client.return_value = mock_client - result = await service.cancel_transaction('99999') + result = await service.cancel_transaction("99999") assert result is False - + @pytest.mark.asyncio async def test_cancel_transaction_not_pending(self, service, mock_transaction_data): """Test cancelling already processed transaction. The API handles validation - we just need to simulate a failure response. """ - with patch.object(service, 'get_client', new_callable=AsyncMock) as mock_get_client: + with patch.object( + service, "get_client", new_callable=AsyncMock + ) as mock_get_client: mock_client = AsyncMock() mock_client.patch.return_value = None # API returns None on failure mock_get_client.return_value = mock_client - result = await service.cancel_transaction('27787') + result = await service.cancel_transaction("27787") assert result is False - + @pytest.mark.asyncio async def test_cancel_transaction_exception_handling(self, service): """Test transaction cancellation exception handling. @@ -300,106 +372,134 @@ class TestTransactionService: When the API call raises an exception, cancel_transaction catches it and returns False. """ - with patch.object(service, 'get_client', new_callable=AsyncMock) as mock_get_client: + with patch.object( + service, "get_client", new_callable=AsyncMock + ) as mock_get_client: mock_client = AsyncMock() mock_client.patch.side_effect = Exception("Database error") mock_get_client.return_value = mock_client - result = await service.cancel_transaction('27787') + result = await service.cancel_transaction("27787") assert result is False - + @pytest.mark.asyncio async def test_get_contested_transactions(self, service, mock_transaction_data): """Test getting contested transactions.""" # Create transactions where multiple teams want the same player contested_data = [ - {**mock_transaction_data, 'id': 1, 'newteam': {'id': 499, 'abbrev': 'WV', 'sname': 'Black Bears', 'lname': 'West Virginia Black Bears', 'season': 12}}, - {**mock_transaction_data, 'id': 2, 'newteam': {'id': 502, 'abbrev': 'LAA', 'sname': 'Angels', 'lname': 'Los Angeles Angels', 'season': 12}}, # Same player, different team + { + **mock_transaction_data, + "id": 1, + "newteam": { + "id": 499, + "abbrev": "WV", + "sname": "Black Bears", + "lname": "West Virginia Black Bears", + "season": 12, + }, + }, + { + **mock_transaction_data, + "id": 2, + "newteam": { + "id": 502, + "abbrev": "LAA", + "sname": "Angels", + "lname": "Los Angeles Angels", + "season": 12, + }, + }, # Same player, different team ] - - with patch.object(service, 'get_all_items', new_callable=AsyncMock) as mock_get: - mock_get.return_value = [Transaction.from_api_data(tx) for tx in contested_data] - + + with patch.object(service, "get_all_items", new_callable=AsyncMock) as mock_get: + mock_get.return_value = [ + Transaction.from_api_data(tx) for tx in contested_data + ] + result = await service.get_contested_transactions(12, 10) - + # Should return both transactions since they're for the same player assert len(result) == 2 - + # Verify API call was made mock_get.assert_called_once() # Note: This test might need adjustment based on actual contested transaction logic - + @pytest.mark.asyncio async def test_api_exception_handling(self, service): """Test API exception handling in service methods.""" - with patch.object(service, 'get_all_items', new_callable=AsyncMock) as mock_get: + with patch.object(service, "get_all_items", new_callable=AsyncMock) as mock_get: mock_get.side_effect = APIException("API unavailable") - + with pytest.raises(APIException): - await service.get_team_transactions('WV', 12) - + await service.get_team_transactions("WV", 12) + def test_global_service_instance(self): """Test that global service instance is properly initialized.""" assert isinstance(transaction_service, TransactionService) assert transaction_service.model_class == Transaction - assert transaction_service.endpoint == 'transactions' + assert transaction_service.endpoint == "transactions" class TestTransactionServiceIntegration: """Integration tests for TransactionService with real-like scenarios.""" - + @pytest.mark.asyncio async def test_full_transaction_workflow(self): """Test complete transaction workflow simulation.""" service = TransactionService() - + # Mock data for a complete workflow mock_data = { - 'id': 27787, - 'week': 10, - 'season': 12, - 'moveid': 'Season-012-Week-10-19-13:04:41', - 'player': { - 'id': 12472, - 'name': 'Test Player', - 'wara': 2.47, - 'season': 12, - 'pos_1': 'LF' + "id": 27787, + "week": 10, + "season": 12, + "moveid": "Season-012-Week-10-19-13:04:41", + "player": { + "id": 12472, + "name": "Test Player", + "wara": 2.47, + "season": 12, + "pos_1": "LF", }, - 'oldteam': { - 'id': 508, - 'abbrev': 'NYD', - 'sname': 'Diamonds', - 'lname': 'New York Diamonds', - 'season': 12 + "oldteam": { + "id": 508, + "abbrev": "NYD", + "sname": "Diamonds", + "lname": "New York Diamonds", + "season": 12, }, - 'newteam': { - 'id': 499, - 'abbrev': 'WV', - 'sname': 'Black Bears', - 'lname': 'West Virginia Black Bears', - 'season': 12 + "newteam": { + "id": 499, + "abbrev": "WV", + "sname": "Black Bears", + "lname": "West Virginia Black Bears", + "season": 12, }, - 'cancelled': False, - 'frozen': False + "cancelled": False, + "frozen": False, } - + # Mock the full workflow properly transaction = Transaction.from_api_data(mock_data) # Mock get_pending_transactions to return our test transaction - with patch.object(service, 'get_pending_transactions', new_callable=AsyncMock) as mock_get_pending: + with patch.object( + service, "get_pending_transactions", new_callable=AsyncMock + ) as mock_get_pending: mock_get_pending.return_value = [transaction] # Mock cancel_transaction - with patch.object(service, 'get_client', new_callable=AsyncMock) as mock_get_client: + with patch.object( + service, "get_client", new_callable=AsyncMock + ) as mock_get_client: mock_client = AsyncMock() mock_client.patch.return_value = "Updated 1 transactions" mock_get_client.return_value = mock_client # Test workflow: get pending -> validate -> cancel - pending = await service.get_pending_transactions('WV', 12) + pending = await service.get_pending_transactions("WV", 12) assert len(pending) == 1 validation = await service.validate_transaction(pending[0]) @@ -407,61 +507,62 @@ class TestTransactionServiceIntegration: cancelled = await service.cancel_transaction(str(pending[0].id)) assert cancelled is True - + @pytest.mark.asyncio async def test_performance_with_large_dataset(self): """Test service performance with large transaction dataset.""" service = TransactionService() - + # Create 100 mock transactions large_dataset = [] for i in range(100): tx_data = { - 'id': i, - 'week': (i % 18) + 1, # Weeks 1-18 - 'season': 12, - 'moveid': f'Season-012-Week-{(i % 18) + 1:02d}-{i}', - 'player': { - 'id': i + 1000, - 'name': f'Player {i}', - 'wara': round(1.0 + (i % 50) * 0.1, 2), - 'season': 12, - 'pos_1': 'LF' + "id": i, + "week": (i % 18) + 1, # Weeks 1-18 + "season": 12, + "moveid": f"Season-012-Week-{(i % 18) + 1:02d}-{i}", + "player": { + "id": i + 1000, + "name": f"Player {i}", + "wara": round(1.0 + (i % 50) * 0.1, 2), + "season": 12, + "pos_1": "LF", }, - 'oldteam': { - 'id': 508, - 'abbrev': 'NYD', - 'sname': 'Diamonds', - 'lname': 'New York Diamonds', - 'season': 12 + "oldteam": { + "id": 508, + "abbrev": "NYD", + "sname": "Diamonds", + "lname": "New York Diamonds", + "season": 12, }, - 'newteam': { - 'id': 499, - 'abbrev': 'WV', - 'sname': 'Black Bears', - 'lname': 'West Virginia Black Bears', - 'season': 12 + "newteam": { + "id": 499, + "abbrev": "WV", + "sname": "Black Bears", + "lname": "West Virginia Black Bears", + "season": 12, }, - 'cancelled': i % 10 == 0, # Every 10th transaction is cancelled - 'frozen': i % 7 == 0 # Every 7th transaction is frozen + "cancelled": i % 10 == 0, # Every 10th transaction is cancelled + "frozen": i % 7 == 0, # Every 7th transaction is frozen } large_dataset.append(Transaction.from_api_data(tx_data)) - - with patch.object(service, 'get_all_items', new_callable=AsyncMock) as mock_get: + + with patch.object(service, "get_all_items", new_callable=AsyncMock) as mock_get: mock_get.return_value = large_dataset - + # Test that service handles large datasets efficiently import time + start_time = time.time() - - result = await service.get_team_transactions('WV', 12) - + + result = await service.get_team_transactions("WV", 12) + end_time = time.time() processing_time = end_time - start_time - + assert len(result) == 100 assert processing_time < 1.0 # Should process quickly - + # Verify sorting worked correctly for i in range(len(result) - 1): - assert result[i].week <= result[i + 1].week \ No newline at end of file + assert result[i].week <= result[i + 1].week