Refactor tests and improve error handling across various services
All checks were successful
Deploy to Test Environment / deploy-to-test (push) Successful in 10m38s

- Updated `useAuth` tests to use async functions for JSON responses to avoid promise resolution issues.
- Changed `AdminBrandManager` tests to use `mockImplementation` for consistent mock behavior.
- Enhanced `ProfileManager.Authenticated` tests to ensure proper error handling and assertions for partial updates.
- Modified `SystemCheck` tests to prevent memory leaks by using `mockImplementation` for API calls.
- Improved error handling in `ai.routes.ts` by refining validation schemas and adding error extraction utility.
- Updated `auth.routes.test.ts` to inject mock logger for better error tracking.
- Refined `flyer.routes.ts` to ensure proper validation and error handling for flyer ID parameters.
- Enhanced `admin.db.ts` to ensure specific errors are re-thrown for better error management.
- Updated `budget.db.test.ts` to improve mock behavior and ensure accurate assertions.
- Refined `flyer.db.ts` to improve error handling for race conditions during store creation.
- Enhanced `notification.db.test.ts` to ensure specific error types are tested correctly.
- Updated `recipe.db.test.ts` to ensure proper handling of not found errors.
- Improved `user.db.ts` to ensure consistent error handling for user retrieval.
- Enhanced `flyerProcessingService.server.test.ts` to ensure accurate assertions on transformed data.
- Updated `logger.server.ts` to disable transport in test environments to prevent issues.
- Refined `queueService.workers.test.ts` to ensure accurate mocking of email service.
- Improved `userService.test.ts` to ensure proper mock implementations for repository classes.
- Enhanced `checksum.test.ts` to ensure reliable file content creation in tests.
- Updated `pdfConverter.test.ts` to reset shared state objects and mock implementations before each test.
This commit is contained in:
2025-12-15 16:40:13 -08:00
parent 0c590675b3
commit d5f185ad99
32 changed files with 430 additions and 182 deletions

View File

@@ -347,6 +347,9 @@ export class AdminRepository {
logger.info(`Successfully resolved unmatched item ${unmatchedFlyerItemId} to master item ${masterItemId}.`);
});
} catch (error) {
if (error instanceof NotFoundError) {
throw error;
}
logger.error({ err: error, unmatchedFlyerItemId, masterItemId }, 'Database transaction error in resolveUnmatchedFlyerItem');
throw new Error('Failed to resolve unmatched flyer item.');
}

View File

@@ -27,17 +27,19 @@ vi.mock('./connection.db', async (importOriginal) => {
return { ...actual, withTransaction: vi.fn() };
});
// Mock the gamification repository, as createBudget calls it.
vi.mock('./gamification.db', () => ({
GamificationRepository: class { awardAchievement = vi.fn(); },
const { mockedAwardAchievement } = vi.hoisted(() => ({
mockedAwardAchievement: vi.fn(),
}));
import { withTransaction } from './connection.db';
// Mock the gamification repository, as createBudget calls it.
vi.mock('./gamification.db', () => ({
GamificationRepository: class { awardAchievement = vi.fn(); },
GamificationRepository: class {
awardAchievement = mockedAwardAchievement;
},
}));
import { withTransaction } from './connection.db';
describe('Budget DB Service', () => {
let budgetRepo: BudgetRepository;
@@ -92,9 +94,8 @@ describe('Budget DB Service', () => {
const result = await budgetRepo.createBudget('user-123', budgetData, mockLogger);
// Now we can assert directly on the mockClient we created.
const { GamificationRepository } = await import('./gamification.db');
expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining('INSERT INTO public.budgets'), expect.any(Array));
expect(GamificationRepository.prototype.awardAchievement).toHaveBeenCalledWith('user-123', 'First Budget Created', mockLogger);
expect(mockedAwardAchievement).toHaveBeenCalledWith('user-123', 'First Budget Created', mockLogger);
expect(result).toEqual(mockCreatedBudget);
expect(withTransaction).toHaveBeenCalledTimes(1);
});
@@ -119,16 +120,19 @@ describe('Budget DB Service', () => {
const budgetData = { name: 'Groceries', amount_cents: 50000, period: 'monthly' as const, start_date: '2024-01-01' };
const mockCreatedBudget: Budget = { budget_id: 1, user_id: 'user-123', ...budgetData };
const achievementError = new Error('Achievement award failed');
mockedAwardAchievement.mockRejectedValueOnce(achievementError);
vi.mocked(withTransaction).mockImplementation(async (callback) => {
const mockClient = { query: vi.fn() };
(mockClient.query as Mock)
.mockResolvedValueOnce({ rows: [mockCreatedBudget] }) // INSERT...RETURNING
.mockRejectedValueOnce(achievementError); // award_achievement fails
.mockResolvedValueOnce({ rows: [mockCreatedBudget] }); // INSERT...RETURNING
await expect(callback(mockClient as unknown as PoolClient)).rejects.toThrow(achievementError);
throw achievementError; // Re-throw for the outer expect
});
await expect(budgetRepo.createBudget('user-123', budgetData, mockLogger)).rejects.toThrow('Failed to create budget.'); // This was a duplicate, fixed.
await expect(budgetRepo.createBudget('user-123', budgetData, mockLogger)).rejects.toThrow('Failed to create budget.');
expect(mockLogger.error).toHaveBeenCalledWith({ err: achievementError, budgetData, userId: 'user-123' }, 'Database error in createBudget');
});

View File

@@ -86,6 +86,7 @@ export class BudgetRepository {
if (res.rowCount === 0) throw new NotFoundError('Budget not found or user does not have permission to update.');
return res.rows[0];
} catch (error) {
if (error instanceof NotFoundError) throw error;
logger.error({ err: error, budgetId, userId }, 'Database error in updateBudget');
throw new Error('Failed to update budget.');
}
@@ -103,6 +104,7 @@ export class BudgetRepository {
throw new NotFoundError('Budget not found or user does not have permission to delete.');
}
} catch (error) {
if (error instanceof NotFoundError) throw error;
logger.error({ err: error, budgetId, userId }, 'Database error in deleteBudget');
throw new Error('Failed to delete budget.');
}

View File

@@ -256,8 +256,10 @@ describe('Flyer DB Service', () => {
});
// The transactional function re-throws the original error from the failed step.
await expect(createFlyerAndItems(flyerData, itemsData, mockLogger)).rejects.toThrow(dbError);
expect(mockLogger.error).toHaveBeenCalledWith({ err: dbError }, 'Database transaction error in createFlyerAndItems');
// Since insertFlyer wraps errors, we expect the wrapped error message.
await expect(createFlyerAndItems(flyerData, itemsData, mockLogger)).rejects.toThrow('Failed to insert flyer into database.');
// The error object passed to the logger will be the wrapped Error object, not the original dbError
expect(mockLogger.error).toHaveBeenCalledWith({ err: expect.any(Error) }, 'Database transaction error in createFlyerAndItems');
expect(withTransaction).toHaveBeenCalledTimes(1);
});
});
@@ -467,7 +469,7 @@ describe('Flyer DB Service', () => {
vi.mocked(withTransaction).mockImplementation(cb => cb(mockClient as unknown as PoolClient));
await expect(flyerRepo.deleteFlyer(999, mockLogger)).rejects.toThrow('Failed to delete flyer.');
expect(mockLogger.error).toHaveBeenCalledWith({ err: expect.any(NotFoundError) }, 'Database transaction error in deleteFlyer');
expect(mockLogger.error).toHaveBeenCalledWith({ err: expect.any(NotFoundError), flyerId: 999 }, 'Database transaction error in deleteFlyer');
});
it('should rollback transaction on generic error', async () => {
@@ -477,7 +479,7 @@ describe('Flyer DB Service', () => {
});
await expect(flyerRepo.deleteFlyer(42, mockLogger)).rejects.toThrow('Failed to delete flyer.'); // This was a duplicate, fixed.
expect(mockLogger.error).toHaveBeenCalledWith({ err: dbError }, 'Database transaction error in deleteFlyer');
expect(mockLogger.error).toHaveBeenCalledWith({ err: dbError, flyerId: 42 }, 'Database transaction error in deleteFlyer');
});
});
});

View File

@@ -36,9 +36,14 @@ export class FlyerRepository {
// Check for a unique constraint violation on name, which could happen in a race condition
// if two processes try to create the same store at the same time.
if (error instanceof Error && 'code' in error && error.code === '23505') {
logger.warn({ storeName }, `Race condition avoided: Store was created by another process. Refetching.`);
const result = await this.db.query<{ store_id: number }>('SELECT store_id FROM public.stores WHERE name = $1', [storeName]);
if (result.rows.length > 0) return result.rows[0].store_id;
try {
logger.warn({ storeName }, `Race condition avoided: Store was created by another process. Refetching.`);
const result = await this.db.query<{ store_id: number }>('SELECT store_id FROM public.stores WHERE name = $1', [storeName]);
if (result.rows.length > 0) return result.rows[0].store_id;
} catch (recoveryError) {
// If recovery fails, log a warning and fall through to the generic error handler
logger.warn({ err: recoveryError, storeName }, 'Race condition recovery failed');
}
}
logger.error({ err: error, storeName }, 'Database error in findOrCreateStore');
throw new Error('Failed to find or create store in database.');

View File

@@ -152,8 +152,8 @@ describe('Notification DB Service', () => {
});
it('should re-throw the specific "not found" error if it occurs', async () => {
// This tests the `if (error instanceof Error && error.message.startsWith('Notification not found'))` line
const notFoundError = new Error('Notification not found or user does not have permission.');
// This tests the `if (error instanceof NotFoundError)` line
const notFoundError = new NotFoundError('Notification not found or user does not have permission.');
mockPoolInstance.query.mockImplementation(() => {
throw notFoundError;
});

View File

@@ -240,7 +240,7 @@ describe('Recipe DB Service', () => {
});
it('should throw NotFoundError if recipe is not found', async () => {
mockQuery.mockResolvedValue({ rows: [] });
mockQuery.mockResolvedValue({ rows: [], rowCount: 0 });
await expect(recipeRepo.getRecipeById(999, mockLogger)).rejects.toThrow('Recipe with ID 999 not found');
});

View File

@@ -92,6 +92,9 @@ export class RecipeRepository {
}
return res.rows[0];
} catch (error) {
if (error instanceof UniqueConstraintError) {
throw error;
}
logger.error({ err: error, userId, recipeId }, 'Database error in addFavoriteRecipe');
if (error instanceof Error && 'code' in error && error.code === '23503') {
throw new ForeignKeyConstraintError('The specified user or recipe does not exist.');

View File

@@ -178,11 +178,12 @@ export class UserRepository {
'SELECT user_id, email, password_hash FROM public.users WHERE user_id = $1',
[userId]
);
if (res.rowCount === 0) {
if ((res.rowCount ?? 0) === 0) {
throw new NotFoundError(`User with ID ${userId} not found.`);
}
return res.rows[0];
} catch (error) {
if (error instanceof NotFoundError) throw error;
logger.error({ err: error, userId }, 'Database error in findUserWithPasswordHashById');
throw new Error('Failed to retrieve user with sensitive data by ID from database.');
}
@@ -366,7 +367,7 @@ export class UserRepository {
'SELECT user_id, email FROM public.users WHERE refresh_token = $1',
[refreshToken]
);
if (res.rowCount === 0) {
if ((res.rowCount ?? 0) === 0) {
throw new NotFoundError('User not found for the given refresh token.');
}
return res.rows[0];