Compare commits
12 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a911224fb4 | ||
|
|
bf4bcef890 | ||
| ac6cd2e0a1 | |||
| eea03880c1 | |||
|
|
7fc263691f | ||
| c0912d36d5 | |||
| 612c2b5943 | |||
|
|
8e787ddcf0 | ||
| 11c52d284c | |||
|
|
b528bd3651 | ||
| 4c5ceb1bd6 | |||
| bcc4ad64dc |
@@ -119,6 +119,11 @@ jobs:
|
||||
# --- JWT Secret for Passport authentication in tests ---
|
||||
JWT_SECRET: ${{ secrets.JWT_SECRET }}
|
||||
|
||||
# --- V8 Coverage for Server Process ---
|
||||
# This variable tells the Node.js process (our server, started by globalSetup)
|
||||
# where to output its raw V8 coverage data.
|
||||
NODE_V8_COVERAGE: '.coverage/tmp/integration-server'
|
||||
|
||||
# --- Increase Node.js memory limit to prevent heap out of memory errors ---
|
||||
# This is crucial for memory-intensive tasks like running tests and coverage.
|
||||
NODE_OPTIONS: '--max-old-space-size=8192'
|
||||
@@ -137,15 +142,15 @@ jobs:
|
||||
# The `|| true` ensures the workflow continues even if tests fail, allowing coverage to run.
|
||||
echo "--- Running Unit Tests ---"
|
||||
# npm run test:unit -- --coverage --reporter=verbose --includeTaskLocation --testTimeout=10000 --silent=passed-only || true
|
||||
npm run test:unit -- --coverage --reporter=verbose --includeTaskLocation --testTimeout=10000 --silent=passed-only --no-file-parallelism || true
|
||||
npm run test:unit -- --coverage --coverage.exclude='**/*.test.ts' --coverage.exclude='**/tests/**' --coverage.exclude='**/mocks/**' --reporter=verbose --includeTaskLocation --testTimeout=10000 --silent=passed-only --no-file-parallelism || true
|
||||
|
||||
echo "--- Running Integration Tests ---"
|
||||
npm run test:integration -- --coverage --reporter=verbose --includeTaskLocation --testTimeout=10000 --silent=passed-only || true
|
||||
npm run test:integration -- --coverage --coverage.exclude='**/*.test.ts' --coverage.exclude='**/tests/**' --coverage.exclude='**/mocks/**' --reporter=verbose --includeTaskLocation --testTimeout=10000 --silent=passed-only || true
|
||||
|
||||
echo "--- Running E2E Tests ---"
|
||||
# Run E2E tests using the dedicated E2E config which inherits from integration config.
|
||||
# We still pass --coverage to enable it, but directory and timeout are now in the config.
|
||||
npx vitest run --config vitest.config.e2e.ts --coverage --reporter=verbose --no-file-parallelism || true
|
||||
npx vitest run --config vitest.config.e2e.ts --coverage --coverage.exclude='**/*.test.ts' --coverage.exclude='**/tests/**' --coverage.exclude='**/mocks/**' --reporter=verbose --no-file-parallelism || true
|
||||
|
||||
# Re-enable secret masking for subsequent steps.
|
||||
echo "::secret-masking::"
|
||||
@@ -174,7 +179,7 @@ jobs:
|
||||
# Run c8: read raw files from the temp dir, and output an Istanbul JSON report.
|
||||
# We only generate the 'json' report here because it's all nyc needs for merging.
|
||||
echo "Server coverage report about to be generated..."
|
||||
npx c8 report --reporter=json --temp-directory .coverage/tmp/integration-server --reports-dir .coverage/integration-server
|
||||
npx c8 report --exclude='**/*.test.ts' --exclude='**/tests/**' --exclude='**/mocks/**' --reporter=json --temp-directory .coverage/tmp/integration-server --reports-dir .coverage/integration-server
|
||||
echo "Server coverage report generated. Verifying existence:"
|
||||
ls -l .coverage/integration-server/coverage-final.json
|
||||
|
||||
@@ -213,7 +218,10 @@ jobs:
|
||||
--reporter=text \
|
||||
--reporter=html \
|
||||
--report-dir .coverage/ \
|
||||
--temp-dir "$NYC_SOURCE_DIR"
|
||||
--temp-dir "$NYC_SOURCE_DIR" \
|
||||
--exclude "**/*.test.ts" \
|
||||
--exclude "**/tests/**" \
|
||||
--exclude "**/mocks/**"
|
||||
|
||||
# Re-enable secret masking for subsequent steps.
|
||||
echo "::secret-masking::"
|
||||
@@ -360,7 +368,7 @@ jobs:
|
||||
|
||||
echo "Installing production dependencies and restarting test server..."
|
||||
cd /var/www/flyer-crawler-test.projectium.com
|
||||
npm install --omit=dev # Install only production dependencies
|
||||
npm install --omit=dev
|
||||
# Use `startOrReload` with the ecosystem file. This is the standard, idempotent way to deploy.
|
||||
# It will START the process if it's not running, or RELOAD it if it is.
|
||||
# We also add `&& pm2 save` to persist the process list across server reboots.
|
||||
|
||||
@@ -21,10 +21,17 @@ module.exports = {
|
||||
},
|
||||
// Test Environment Settings
|
||||
env_test: {
|
||||
NODE_ENV: 'development', // Use 'development' for test to enable more verbose logging if needed
|
||||
NODE_ENV: 'test', // Set to 'test' to match the environment purpose and disable pino-pretty
|
||||
name: 'flyer-crawler-api-test',
|
||||
cwd: '/var/www/flyer-crawler-test.projectium.com',
|
||||
},
|
||||
// Development Environment Settings
|
||||
env_development: {
|
||||
NODE_ENV: 'development',
|
||||
name: 'flyer-crawler-api-dev',
|
||||
watch: true,
|
||||
ignore_watch: ['node_modules', 'logs', '*.log', 'flyer-images', '.git'],
|
||||
},
|
||||
},
|
||||
{
|
||||
// --- General Worker ---
|
||||
@@ -39,10 +46,17 @@ module.exports = {
|
||||
},
|
||||
// Test Environment Settings
|
||||
env_test: {
|
||||
NODE_ENV: 'development',
|
||||
NODE_ENV: 'test',
|
||||
name: 'flyer-crawler-worker-test',
|
||||
cwd: '/var/www/flyer-crawler-test.projectium.com',
|
||||
},
|
||||
// Development Environment Settings
|
||||
env_development: {
|
||||
NODE_ENV: 'development',
|
||||
name: 'flyer-crawler-worker-dev',
|
||||
watch: true,
|
||||
ignore_watch: ['node_modules', 'logs', '*.log', 'flyer-images', '.git'],
|
||||
},
|
||||
},
|
||||
{
|
||||
// --- Analytics Worker ---
|
||||
@@ -57,10 +71,17 @@ module.exports = {
|
||||
},
|
||||
// Test Environment Settings
|
||||
env_test: {
|
||||
NODE_ENV: 'development',
|
||||
NODE_ENV: 'test',
|
||||
name: 'flyer-crawler-analytics-worker-test',
|
||||
cwd: '/var/www/flyer-crawler-test.projectium.com',
|
||||
},
|
||||
// Development Environment Settings
|
||||
env_development: {
|
||||
NODE_ENV: 'development',
|
||||
name: 'flyer-crawler-analytics-worker-dev',
|
||||
watch: true,
|
||||
ignore_watch: ['node_modules', 'logs', '*.log', 'flyer-images', '.git'],
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
4
package-lock.json
generated
4
package-lock.json
generated
@@ -1,12 +1,12 @@
|
||||
{
|
||||
"name": "flyer-crawler",
|
||||
"version": "0.0.26",
|
||||
"version": "0.1.0",
|
||||
"lockfileVersion": 3,
|
||||
"requires": true,
|
||||
"packages": {
|
||||
"": {
|
||||
"name": "flyer-crawler",
|
||||
"version": "0.0.26",
|
||||
"version": "0.1.0",
|
||||
"dependencies": {
|
||||
"@bull-board/api": "^6.14.2",
|
||||
"@bull-board/express": "^6.14.2",
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "flyer-crawler",
|
||||
"private": true,
|
||||
"version": "0.0.26",
|
||||
"version": "0.1.0",
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"dev": "concurrently \"npm:start:dev\" \"vite\"",
|
||||
|
||||
@@ -353,10 +353,11 @@ describe('AI Routes (/api/ai)', () => {
|
||||
expect(response.status).toBe(400);
|
||||
});
|
||||
|
||||
it('should return 409 Conflict if flyer checksum already exists', async () => {
|
||||
it('should return 409 Conflict and delete the uploaded file if flyer checksum already exists', async () => {
|
||||
// Arrange
|
||||
const mockExistingFlyer = createMockFlyer({ flyer_id: 99 });
|
||||
vi.mocked(mockedDb.flyerRepo.findFlyerByChecksum).mockResolvedValue(mockExistingFlyer); // Duplicate found
|
||||
const unlinkSpy = vi.spyOn(fs.promises, 'unlink').mockResolvedValue(undefined);
|
||||
|
||||
// Act
|
||||
const response = await supertest(app)
|
||||
@@ -368,6 +369,10 @@ describe('AI Routes (/api/ai)', () => {
|
||||
expect(response.status).toBe(409);
|
||||
expect(response.body.message).toBe('This flyer has already been processed.');
|
||||
expect(mockedDb.createFlyerAndItems).not.toHaveBeenCalled();
|
||||
// Assert that the file was deleted
|
||||
expect(unlinkSpy).toHaveBeenCalledTimes(1);
|
||||
// The filename is predictable in the test environment because of the multer config in ai.routes.ts
|
||||
expect(unlinkSpy).toHaveBeenCalledWith(expect.stringContaining('flyerImage-test-flyer-image.jpg'));
|
||||
});
|
||||
|
||||
it('should accept payload when extractedData.items is missing and save with empty items', async () => {
|
||||
|
||||
@@ -50,6 +50,15 @@ const errMsg = (e: unknown) => {
|
||||
return String(e || 'An unknown error occurred.');
|
||||
};
|
||||
|
||||
const cleanupUploadedFile = async (file?: Express.Multer.File) => {
|
||||
if (!file) return;
|
||||
try {
|
||||
await fs.promises.unlink(file.path);
|
||||
} catch (err) {
|
||||
// Ignore cleanup errors (e.g. file already deleted)
|
||||
}
|
||||
};
|
||||
|
||||
const cropAreaObjectSchema = z.object({
|
||||
x: z.number(),
|
||||
y: z.number(),
|
||||
@@ -185,7 +194,7 @@ router.use((req: Request, res: Response, next: NextFunction) => {
|
||||
'[API /ai] Incoming request',
|
||||
);
|
||||
} catch (e: unknown) {
|
||||
logger.error({ error: e }, 'Failed to log incoming AI request headers');
|
||||
logger.error({ error: errMsg(e) }, 'Failed to log incoming AI request headers');
|
||||
}
|
||||
next();
|
||||
});
|
||||
@@ -316,7 +325,7 @@ router.post(
|
||||
|
||||
// Try several ways to obtain the payload so we are tolerant to client variations.
|
||||
let parsed: FlyerProcessPayload = {};
|
||||
let extractedData: Partial<ExtractedCoreData> = {};
|
||||
let extractedData: Partial<ExtractedCoreData> | null | undefined = {};
|
||||
try {
|
||||
// If the client sent a top-level `data` field (stringified JSON), parse it.
|
||||
if (req.body && (req.body.data || req.body.extractedData)) {
|
||||
@@ -337,7 +346,7 @@ router.post(
|
||||
) as FlyerProcessPayload;
|
||||
}
|
||||
// If parsed itself contains an `extractedData` field, use that, otherwise assume parsed is the extractedData
|
||||
extractedData = parsed.extractedData ?? (parsed as Partial<ExtractedCoreData>);
|
||||
extractedData = 'extractedData' in parsed ? parsed.extractedData : (parsed as Partial<ExtractedCoreData>);
|
||||
} else {
|
||||
// No explicit `data` field found. Attempt to interpret req.body as an object (Express may have parsed multipart fields differently).
|
||||
try {
|
||||
@@ -383,6 +392,12 @@ router.post(
|
||||
|
||||
// Pull common metadata fields (checksum, originalFileName) from whichever shape we parsed.
|
||||
const checksum = parsed.checksum ?? parsed?.data?.checksum ?? '';
|
||||
|
||||
if (!checksum) {
|
||||
await cleanupUploadedFile(req.file);
|
||||
return res.status(400).json({ message: 'Checksum is required.' });
|
||||
}
|
||||
|
||||
const originalFileName =
|
||||
parsed.originalFileName ?? parsed?.data?.originalFileName ?? req.file.originalname;
|
||||
const userProfile = req.user as UserProfile | undefined;
|
||||
@@ -429,6 +444,7 @@ router.post(
|
||||
const existingFlyer = await db.flyerRepo.findFlyerByChecksum(checksum, req.log);
|
||||
if (existingFlyer) {
|
||||
logger.warn(`Duplicate flyer upload attempt blocked for checksum: ${checksum}`);
|
||||
await cleanupUploadedFile(req.file);
|
||||
return res.status(409).json({ message: 'This flyer has already been processed.' });
|
||||
}
|
||||
|
||||
@@ -476,6 +492,7 @@ router.post(
|
||||
|
||||
res.status(201).json({ message: 'Flyer processed and saved successfully.', flyer: newFlyer });
|
||||
} catch (error) {
|
||||
await cleanupUploadedFile(req.file);
|
||||
next(error);
|
||||
}
|
||||
},
|
||||
|
||||
@@ -96,7 +96,8 @@ describe('Price Routes (/api/price-history)', () => {
|
||||
.send({ masterItemIds: 'not-an-array' });
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(response.body.errors[0].message).toContain('Expected array, received string');
|
||||
// The actual message is "Invalid input: expected array, received string"
|
||||
expect(response.body.errors[0].message).toBe('Invalid input: expected array, received string');
|
||||
});
|
||||
|
||||
it('should return 400 if masterItemIds contains non-positive integers', async () => {
|
||||
@@ -112,7 +113,8 @@ describe('Price Routes (/api/price-history)', () => {
|
||||
const response = await supertest(app).post('/api/price-history').send({});
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(response.body.errors[0].message).toBe('Required');
|
||||
// The actual message is "Invalid input: expected array, received undefined"
|
||||
expect(response.body.errors[0].message).toBe('Invalid input: expected array, received undefined');
|
||||
});
|
||||
|
||||
it('should return 400 for invalid limit and offset', async () => {
|
||||
@@ -122,8 +124,9 @@ describe('Price Routes (/api/price-history)', () => {
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(response.body.errors).toHaveLength(2);
|
||||
expect(response.body.errors[0].message).toBe('Number must be greater than 0');
|
||||
expect(response.body.errors[1].message).toBe('Expected number, received string');
|
||||
// The actual message is "Too small: expected number to be >0"
|
||||
expect(response.body.errors[0].message).toBe('Too small: expected number to be >0');
|
||||
expect(response.body.errors[1].message).toBe('Invalid input: expected number, received NaN');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -34,6 +34,9 @@ export const logger = pino({
|
||||
'*.body.password',
|
||||
'*.body.newPassword',
|
||||
'*.body.currentPassword',
|
||||
'*.body.confirmPassword',
|
||||
'*.body.refreshToken',
|
||||
'*.body.token',
|
||||
],
|
||||
censor: '[REDACTED]',
|
||||
},
|
||||
|
||||
@@ -59,7 +59,7 @@ describe('Zod Utilities', () => {
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
// z.string() will throw its own error message before min(1) is checked.
|
||||
expect(result.error.issues[0].message).toBe('Expected string, received number');
|
||||
expect(result.error.issues[0].message).toBe('Invalid input: expected string, received number');
|
||||
}
|
||||
});
|
||||
|
||||
@@ -67,7 +67,7 @@ describe('Zod Utilities', () => {
|
||||
const result = schema.safeParse({ a: 1 });
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
expect(result.error.issues[0].message).toBe('Expected string, received object');
|
||||
expect(result.error.issues[0].message).toBe('Invalid input: expected string, received object');
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -95,7 +95,7 @@ describe('Zod Utilities', () => {
|
||||
const result = schema.safeParse({ params: { id: 'abc' } });
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
expect(result.error.issues[0].message).toContain('Expected number, received nan');
|
||||
expect(result.error.issues[0].message).toBe('Invalid input: expected number, received NaN');
|
||||
}
|
||||
});
|
||||
|
||||
@@ -103,7 +103,7 @@ describe('Zod Utilities', () => {
|
||||
const result = schema.safeParse({ params: { id: -1 } });
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
expect(result.error.issues[0].message).toContain('Must be a number');
|
||||
expect(result.error.issues[0].message).toBe("Invalid ID for parameter 'id'. Must be a number.");
|
||||
}
|
||||
});
|
||||
|
||||
@@ -111,7 +111,7 @@ describe('Zod Utilities', () => {
|
||||
const result = schema.safeParse({ params: { id: 1.5 } });
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
expect(result.error.issues[0].message).toContain('Must be a number');
|
||||
expect(result.error.issues[0].message).toBe("Invalid ID for parameter 'id'. Must be a number.");
|
||||
}
|
||||
});
|
||||
|
||||
@@ -119,7 +119,7 @@ describe('Zod Utilities', () => {
|
||||
const result = schema.safeParse({ params: { id: 0 } });
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
expect(result.error.issues[0].message).toContain('Must be a number');
|
||||
expect(result.error.issues[0].message).toBe("Invalid ID for parameter 'id'. Must be a number.");
|
||||
}
|
||||
});
|
||||
|
||||
@@ -224,7 +224,7 @@ describe('Zod Utilities', () => {
|
||||
const floatResult = schema.safeParse('123.45');
|
||||
expect(floatResult.success).toBe(false);
|
||||
if (!floatResult.success) {
|
||||
expect(floatResult.error.issues[0].message).toBe('Expected integer, received float');
|
||||
expect(floatResult.error.issues[0].message).toBe('Invalid input: expected int, received number');
|
||||
}
|
||||
});
|
||||
|
||||
@@ -234,7 +234,7 @@ describe('Zod Utilities', () => {
|
||||
const zeroResult = schema.safeParse('0');
|
||||
expect(zeroResult.success).toBe(false);
|
||||
if (!zeroResult.success) {
|
||||
expect(zeroResult.error.issues[0].message).toBe('Number must be greater than 0');
|
||||
expect(zeroResult.error.issues[0].message).toBe('Too small: expected number to be >0');
|
||||
}
|
||||
});
|
||||
|
||||
@@ -244,7 +244,7 @@ describe('Zod Utilities', () => {
|
||||
const negativeResult = schema.safeParse('-1');
|
||||
expect(negativeResult.success).toBe(false);
|
||||
if (!negativeResult.success) {
|
||||
expect(negativeResult.error.issues[0].message).toBe('Number must be greater than or equal to 0');
|
||||
expect(negativeResult.error.issues[0].message).toBe('Too small: expected number to be >=0');
|
||||
}
|
||||
});
|
||||
|
||||
@@ -254,12 +254,12 @@ describe('Zod Utilities', () => {
|
||||
const tooSmallResult = schema.safeParse('9');
|
||||
expect(tooSmallResult.success).toBe(false);
|
||||
if (!tooSmallResult.success) {
|
||||
expect(tooSmallResult.error.issues[0].message).toBe('Number must be greater than or equal to 10');
|
||||
expect(tooSmallResult.error.issues[0].message).toBe('Too small: expected number to be >=10');
|
||||
}
|
||||
const tooLargeResult = schema.safeParse('21');
|
||||
expect(tooLargeResult.success).toBe(false);
|
||||
if (!tooLargeResult.success) {
|
||||
expect(tooLargeResult.error.issues[0].message).toBe('Number must be less than or equal to 20');
|
||||
expect(tooLargeResult.error.issues[0].message).toBe('Too big: expected number to be <=20');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -59,18 +59,27 @@ export const optionalNumeric = (
|
||||
nonnegative?: boolean;
|
||||
} = {},
|
||||
) => {
|
||||
let schema = z.coerce.number();
|
||||
let numberSchema = z.coerce.number();
|
||||
|
||||
if (options.integer) schema = schema.int();
|
||||
if (options.positive) schema = schema.positive();
|
||||
else if (options.nonnegative) schema = schema.nonnegative();
|
||||
if (options.integer) numberSchema = numberSchema.int();
|
||||
if (options.positive) numberSchema = numberSchema.positive();
|
||||
else if (options.nonnegative) numberSchema = numberSchema.nonnegative();
|
||||
|
||||
if (options.min !== undefined) schema = schema.min(options.min);
|
||||
if (options.max !== undefined) schema = schema.max(options.max);
|
||||
if (options.min !== undefined) numberSchema = numberSchema.min(options.min);
|
||||
if (options.max !== undefined) numberSchema = numberSchema.max(options.max);
|
||||
|
||||
if (options.default !== undefined) return schema.optional().default(options.default);
|
||||
// Make the number schema optional *before* preprocessing. This allows it to correctly handle
|
||||
// the `undefined` value that our preprocessor generates from `null`.
|
||||
const optionalNumberSchema = numberSchema.optional();
|
||||
|
||||
return schema.optional();
|
||||
// This is crucial because z.coerce.number(null) results in 0, which bypasses
|
||||
// the .optional() and .default() logic for null inputs. We want null to be
|
||||
// treated as "not provided", just like undefined.
|
||||
const schema = z.preprocess((val) => (val === null ? undefined : val), optionalNumberSchema);
|
||||
|
||||
if (options.default !== undefined) return schema.default(options.default);
|
||||
|
||||
return schema;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user