updated testing guidelines

Author: gallayl

.cursor/rules/TESTING_GUIDELINES.md

@@ -350,6 +350,305 @@ describe('UserService', () => {
 })
 ```
 
+### Error Scenario Testing
+
+**CRITICAL:** All service and component tests MUST include error scenario coverage in addition to happy path tests.
+
+#### Required Error Scenarios
+
+For every service method, test:
+
+1. **API/Network Errors** - Server failures, timeouts, network disconnects
+2. **Validation Errors** - Invalid input data (400 responses)
+3. **Authentication Errors** - Unauthorized access (401 responses)
+4. **Not Found Errors** - Missing resources (404 responses)
+5. **Conflict Errors** - Duplicate resources (409 responses)
+6. **Server Errors** - Internal failures (500 responses)
+
+#### Service Error Testing Pattern
+
+```typescript
+// ✅ Good - testing error scenarios
+import { Injector } from '@furystack/inject'
+import { usingAsync } from '@furystack/utils'
+import { RequestError } from '@furystack/rest-service'
+import { describe, expect, it, vi } from 'vitest'
+
+describe('DashboardService', () => {
+  describe('getDashboard', () => {
+    it('should fetch a dashboard by id', async () => {
+      // Happy path test
+      const mockCall = vi.fn().mockResolvedValue({ result: mockDashboard })
+      // ... test implementation
+    })
+
+    it('should throw RequestError when API returns 404', async () => {
+      const mockCall = vi.fn().mockRejectedValue(
+        new RequestError('Dashboard not found', 404)
+      )
+      const injector = createTestInjector(mockCall)
+
+      await usingAsync(injector, async (i) => {
+        const service = i.getInstance(DashboardService)
+        
+        await expect(service.getDashboard('invalid-id')).rejects.toThrow('Dashboard not found')
+      })
+    })
+
+    it('should handle network errors', async () => {
+      const mockCall = vi.fn().mockRejectedValue(new Error('Network error'))
+      const injector = createTestInjector(mockCall)
+
+      await usingAsync(injector, async (i) => {
+        const service = i.getInstance(DashboardService)
+        
+        await expect(service.getDashboard('dashboard-1')).rejects.toThrow('Network error')
+      })
+    })
+
+    it('should handle server errors', async () => {
+      const mockCall = vi.fn().mockRejectedValue(
+        new RequestError('Internal server error', 500)
+      )
+      const injector = createTestInjector(mockCall)
+
+      await usingAsync(injector, async (i) => {
+        const service = i.getInstance(DashboardService)
+        
+        await expect(service.getDashboard('dashboard-1')).rejects.toThrow('Internal server error')
+      })
+    })
+  })
+
+  describe('createDashboard', () => {
+    it('should throw validation error for invalid data', async () => {
+      const mockCall = vi.fn().mockRejectedValue(
+        new RequestError('Invalid dashboard data', 400)
+      )
+      const injector = createTestInjector(mockCall)
+
+      await usingAsync(injector, async (i) => {
+        const service = i.getInstance(DashboardService)
+        
+        await expect(
+          service.createDashboard({ name: '', owner: '', description: '', widgets: [] })
+        ).rejects.toThrow('Invalid dashboard data')
+      })
+    })
+
+    it('should throw conflict error when dashboard already exists', async () => {
+      const mockCall = vi.fn().mockRejectedValue(
+        new RequestError('Dashboard already exists', 409)
+      )
+      const injector = createTestInjector(mockCall)
+
+      await usingAsync(injector, async (i) => {
+        const service = i.getInstance(DashboardService)
+        
+        await expect(
+          service.createDashboard({ name: 'Existing', owner: 'user', description: '', widgets: [] })
+        ).rejects.toThrow('Dashboard already exists')
+      })
+    })
+  })
+})
+
+// ❌ Avoid - only testing happy paths
+describe('DashboardService', () => {
+  it('should fetch a dashboard by id', async () => {
+    // Only happy path, missing error scenarios
+  })
+})
+```
+
+#### Cache Error State Testing
+
+When testing services with Cache, verify error states are properly handled:
+
+```typescript
+// ✅ Good - testing cache error states
+describe('MovieService', () => {
+  it('should handle cache load errors', async () => {
+    const mockCall = vi.fn().mockRejectedValue(new Error('API unavailable'))
+    const injector = createTestInjector(mockCall)
+
+    await usingAsync(injector, async (i) => {
+      const service = i.getInstance(MovieService)
+      
+      // First call should fail
+      await expect(service.getMovie('tt1234567')).rejects.toThrow('API unavailable')
+      
+      // Verify cache doesn't store failed result
+      mockCall.mockResolvedValue({ result: createMockMovie() })
+      const movie = await service.getMovie('tt1234567')
+      expect(movie).toBeDefined()
+    })
+  })
+})
+```
+
+#### Cache Invalidation Testing
+
+Verify that cache invalidation actually causes fresh data to be fetched:
+
+```typescript
+// ✅ Good - verifying cache invalidation
+describe('DashboardService', () => {
+  it('should fetch fresh data after cache invalidation', async () => {
+    const originalDashboard = createMockDashboard('dashboard-1', 'Original')
+    const updatedDashboard = createMockDashboard('dashboard-1', 'Updated')
+    
+    const mockCall = vi
+      .fn()
+      .mockResolvedValueOnce({ result: originalDashboard })
+      .mockResolvedValueOnce({ result: updatedDashboard })
+      .mockResolvedValueOnce({ result: updatedDashboard })
+    
+    const injector = createTestInjector(mockCall)
+
+    await usingAsync(injector, async (i) => {
+      const service = i.getInstance(DashboardService)
+      
+      // Load initial data (API call #1)
+      const initial = await service.getDashboard('dashboard-1')
+      expect(initial.name).toBe('Original')
+      
+      // Update dashboard (API call #2, invalidates cache)
+      await service.updateDashboard('dashboard-1', { 
+        name: 'Updated',
+        owner: 'user',
+        description: '',
+        widgets: []
+      })
+      
+      // Fetch again should get fresh data (API call #3 due to invalidation)
+      const fresh = await service.getDashboard('dashboard-1')
+      expect(fresh.name).toBe('Updated')
+      expect(mockCall).toHaveBeenCalledTimes(3)
+    })
+  })
+})
+
+// ❌ Avoid - not verifying cache invalidation
+it('should update a dashboard', async () => {
+  await service.updateDashboard('dashboard-1', updates)
+  // Missing: verify that subsequent getDashboard calls fetch fresh data
+})
+```
+
+#### Observable Error State Testing
+
+Test that Observables properly reflect error states:
+
+```typescript
+// ✅ Good - testing Observable error states
+describe('DataService', () => {
+  it('should set error state when load fails', async () => {
+    const mockCall = vi.fn().mockRejectedValue(new Error('Load failed'))
+    const injector = createTestInjector(mockCall)
+
+    await usingAsync(injector, async (i) => {
+      const service = i.getInstance(DataService)
+      const observable = service.getDataAsObservable('data-1')
+      
+      // Track state changes
+      const states: string[] = []
+      observable.subscribe((state) => states.push(state.status))
+      
+      // Trigger load
+      try {
+        await service.getData('data-1')
+      } catch (error) {
+        // Expected to fail
+      }
+      
+      // Verify error state transition
+      expect(states).toContain('error')
+      const currentState = observable.getValue()
+      expect(currentState.status).toBe('error')
+      if (currentState.status === 'error') {
+        expect(currentState.error).toBe('Load failed')
+      }
+    })
+  })
+})
+```
+
+#### Component Error Handling
+
+Test that components handle and display errors appropriately:
+
+```typescript
+// ✅ Good - testing component error handling
+describe('DashboardEditor', () => {
+  it('should display error message when save fails', async () => {
+    const mockService = {
+      updateDashboard: vi.fn().mockRejectedValue(new Error('Save failed'))
+    }
+    
+    const injector = new Injector()
+    injector.setExplicitInstance(mockService, DashboardService)
+    
+    const rootElement = document.getElementById('root') as HTMLDivElement
+    initializeShadeRoot({
+      injector,
+      rootElement,
+      jsxElement: <DashboardEditor dashboardId="dashboard-1" />
+    })
+    
+    // Trigger save
+    const saveButton = rootElement.querySelector('button[type="submit"]') as HTMLButtonElement
+    saveButton.click()
+    
+    // Wait for error to be displayed
+    await new Promise(resolve => setTimeout(resolve, 100))
+    
+    // Verify error display
+    const errorMessage = rootElement.textContent
+    expect(errorMessage).toContain('Save failed')
+  })
+
+  it('should handle loading state during async operations', async () => {
+    const mockService = {
+      getDashboard: vi.fn(() => new Promise(resolve => setTimeout(() => resolve(mockDashboard), 100)))
+    }
+    
+    const injector = new Injector()
+    injector.setExplicitInstance(mockService, DashboardService)
+    
+    const rootElement = document.getElementById('root') as HTMLDivElement
+    initializeShadeRoot({
+      injector,
+      rootElement,
+      jsxElement: <DashboardEditor dashboardId="dashboard-1" />
+    })
+    
+    // Verify loading state is shown
+    expect(rootElement.textContent).toContain('Loading')
+    
+    // Wait for load to complete
+    await new Promise(resolve => setTimeout(resolve, 150))
+    
+    // Verify content is displayed
+    expect(rootElement.textContent).not.toContain('Loading')
+  })
+})
+```
+
+#### Error Testing Checklist
+
+For each service method, ensure you have tests for:
+
+- [ ] Happy path (successful operation)
+- [ ] Network/API errors
+- [ ] Validation errors (400)
+- [ ] Authentication errors (401) if applicable
+- [ ] Not found errors (404)
+- [ ] Conflict errors (409) if applicable
+- [ ] Server errors (500)
+- [ ] Cache invalidation (for update/delete operations)
+- [ ] Observable error states (for methods returning observables)
+
 ### Testing Observable State
 
 ```typescript
@@ -605,16 +904,18 @@ vitest
 
 **Key Principles:**
 
-1. **Minimal mocking** - Only mock what's necessary
-2. **Type-safe mocks** - Always type mock callbacks and data
-3. **Arrange-Act-Assert** - Follow clear test structure
-4. **Semantic locators** - Use accessible queries in E2E tests
-5. **Helper functions** - Encapsulate common workflows
-6. **Test behavior** - Not implementation details
-7. **Co-locate tests** - Keep tests near the code they test
-8. **Descriptive names** - Make test names clear and specific
-9. **Hoisted mocks** - Define mocks at the top of test files
-10. **Independent tests** - Each test should run in isolation
+1. **Error scenarios first** - Test both happy paths AND error cases (CRITICAL)
+2. **Minimal mocking** - Only mock what's necessary
+3. **Type-safe mocks** - Always type mock callbacks and data
+4. **Arrange-Act-Assert** - Follow clear test structure
+5. **Semantic locators** - Use accessible queries in E2E tests
+6. **Helper functions** - Encapsulate common workflows
+7. **Test behavior** - Not implementation details
+8. **Co-locate tests** - Keep tests near the code they test
+9. **Descriptive names** - Make test names clear and specific
+10. **Hoisted mocks** - Define mocks at the top of test files
+11. **Independent tests** - Each test should run in isolation
+12. **Verify side effects** - Test cache invalidation, WebSocket listeners, cleanup
 
 **Testing Checklist:**
 
@@ -624,7 +925,15 @@ vitest
 - [ ] Helper functions for common workflows
 - [ ] Tests are independent and parallelizable
 - [ ] Observable state changes are tested
-- [ ] Error scenarios are covered
+- [ ] **Error scenarios are covered (CRITICAL)**
+  - [ ] API/Network errors tested
+  - [ ] Validation errors tested (400)
+  - [ ] Authentication errors tested (401) if applicable
+  - [ ] Not found errors tested (404)
+  - [ ] Conflict errors tested (409) if applicable
+  - [ ] Server errors tested (500)
+  - [ ] Cache invalidation verified
+  - [ ] Observable error states tested
 - [ ] No brittle CSS selectors
 
 **Tools:**