-
Notifications
You must be signed in to change notification settings - Fork 0
Update linkry edge redirect unit tests #411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe test suite for the CloudFront Lambda@Edge Handler was updated to replace a single DynamoDB success test with two host-specific test scenarios for "go.acm" and "acm.gg". Each test now verifies the complete 302 response structure, including status code, headers, and redirect location. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
💰 Infracost reportMonthly estimate generatedThis comment will be updated when code changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/unit/linkryEdgeFunction/main.test.ts (1)
117-143: LGTM! Good coverage for acm.gg domain.The test properly verifies the redirect behavior for the acm.gg host with comprehensive assertions. This complements the existing acm.gg subdomain tests (lines 246-262) by covering the base domain scenario.
Optional: Consider using parameterized tests (e.g.,
it.each) to reduce duplication between the two similar tests:it.each([ { host: "go.acm.illinois.edu", slug: "test-link" }, { host: "acm.gg", slug: "testgg" }, ])("should return 302 redirect when link is found in DynamoDB for $host", async ({ host, slug }) => { const redirectUrl = "https://example.com/target"; dynamoMock.on(QueryCommand).resolves({ Items: [ { slug: { S: slug }, access: { S: "OWNER#user123" }, redirect: { S: redirectUrl }, }, ], }); const event = createEvent(`/${slug}`, host); const result = await handler(event); assertIsResponse(result); expect(result).toEqual({ status: "302", statusDescription: "Found", headers: { location: [{ key: "Location", value: redirectUrl }], "cache-control": [ { key: "Cache-Control", value: "public, max-age=30" }, ], }, }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
tests/unit/linkryEdgeFunction/main.test.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Application
- GitHub Check: Run Unit Tests
🔇 Additional comments (2)
tests/unit/linkryEdgeFunction/main.test.ts (2)
89-89: Good improvement to test description specificity.The updated test name clearly indicates which host domain is being tested, making the test suite more readable and maintainable.
101-115: LGTM! Enhanced test coverage and clarity.The explicit host parameter and comprehensive response structure assertion significantly improve test quality by verifying the complete 302 redirect behavior rather than partial properties.
Summary by CodeRabbit