✅ server: replace redis mock with in-memory redis in tests#882
✅ server: replace redis mock with in-memory redis in tests#882cruzdanilo merged 7 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 5347cf3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the testing infrastructure by transitioning from a basic Redis mock to an in-memory Redis server. This change ensures that tests interact with a Redis instance that behaves more like a real one, leading to more reliable and accurate test results without the overhead of a full Redis server. The update also involves refactoring the Redis utility module and adjusting test suites to align with the new in-memory setup. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughMigrates tests from an ioredis-mock to a redis-memory-server, refactors Redis exports to provide Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/package.json (1)
69-69: 🛠️ Refactor suggestion | 🟠 MajorRemove stale type package
@types/ioredis-mock.Since
ioredis-mockwas removed in favor ofredis-memory-server, the corresponding type package@types/ioredis-mockis now unused and should be removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 20b390f7-2944-4df0-9a35-42117eb28e2e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.changeset/wise-sites-add.mdserver/Dockerfileserver/api/auth/authentication.tsserver/api/auth/registration.tsserver/hooks/block.tsserver/package.jsonserver/test/api/auth.test.tsserver/test/e2e.tsserver/test/hooks/block.test.tsserver/test/hooks/panda.test.tsserver/test/mocks/redis.tsserver/test/redis.tsserver/utils/redis.tsserver/vitest.config.mts
💤 Files with no reviewable changes (3)
- server/test/hooks/panda.test.ts
- server/test/e2e.ts
- server/test/mocks/redis.ts
| return async function teardown() { | ||
| await server.stop(); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if closeRedis is exported from server/utils/redis.ts
# Expect: closeRedis function to be exported
ast-grep --pattern 'export function closeRedis() {
$$$
}'
# Also check with const export pattern
rg -n "export.*closeRedis" server/utils/redis.tsRepository: exactly/exa
Length of output: 257
🏁 Script executed:
cat -n server/test/redis.tsRepository: exactly/exa
Length of output: 542
Close Redis connections before stopping the server in teardown.
Import closeRedis from server/utils/redis.ts and call it before server.stop() to ensure Redis client connections are properly closed. Without this, connections may be left open or abruptly terminated when the server stops.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #882 +/- ##
==========================================
+ Coverage 71.23% 71.24% +0.01%
==========================================
Files 212 212
Lines 8378 8381 +3
Branches 2741 2741
==========================================
+ Hits 5968 5971 +3
Misses 2132 2132
Partials 278 278
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5347cf38cc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/index.ts (1)
325-331:⚠️ Potential issue | 🟠 MajorAdd a shutdown timeout and preserve failure reasons.
Line 325 waits on multiple external closers without a timeout; one hung promise can block shutdown indefinitely. Also, the generic rejection message loses actionable cause details.
🔧 Proposed fix
export async function close() { return new Promise((resolve, reject) => { server.close((error) => { - Promise.allSettled([closeSentry(), closeRedis(), closeSegment(), database.$client.end()]) + const closing = Promise.allSettled([closeSentry(), closeRedis(), closeSegment(), database.$client.end()]); + const timeout = new Promise<never>((_, timeoutReject) => + setTimeout(() => timeoutReject(new Error("closing services timed out")), 10_000), + ); + Promise.race([closing, timeout]) .then((results) => { if (error) reject(error); - else if (results.some((result) => result.status === "rejected")) reject(new Error("closing services failed")); + else if (Array.isArray(results) && results.some((result) => result.status === "rejected")) { + const reasons = results + .filter((result): result is PromiseRejectedResult => result.status === "rejected") + .map((result) => String(result.reason)) + .join("; "); + reject(new Error(`closing services failed: ${reasons}`)); + } else resolve(null); }) .catch(reject); }); }); }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7534e5f5-73af-4844-8ff5-36bfa822b947
📒 Files selected for processing (2)
.changeset/red-rat-pet.mdserver/index.ts
Summary by CodeRabbit
Tests
Chores
Bug Fixes