Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefines Jest mocking in the S3 uploader tests by giving readdir spies precise promise and argument typings and returning a correctly typed file name array, improving type safety and readability. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The custom typing on the
jest.spyOn(fs.promises, 'readdir')spy narrows the signature to[path: fs.PathLike], which omits the optionaloptionsparameter used in somereaddiroverloads; consider reflecting the full function type (e.g., viajest.MockedFunction<typeof fs.promises.readdir>) so tests stay aligned with the real API surface. - Instead of casting through
unknowntojest.SpyInstance<Promise<string[]>, [fs.PathLike]>, consider defining a small typed helper or using a genericspyOn<typeof fs.promises.readdir>pattern to avoid brittle type assertions and make the intent clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The custom typing on the `jest.spyOn(fs.promises, 'readdir')` spy narrows the signature to `[path: fs.PathLike]`, which omits the optional `options` parameter used in some `readdir` overloads; consider reflecting the full function type (e.g., via `jest.MockedFunction<typeof fs.promises.readdir>`) so tests stay aligned with the real API surface.
- Instead of casting through `unknown` to `jest.SpyInstance<Promise<string[]>, [fs.PathLike]>`, consider defining a small typed helper or using a generic `spyOn<typeof fs.promises.readdir>` pattern to avoid brittle type assertions and make the intent clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2 +/- ##
=======================================
Coverage ? 92.21%
=======================================
Files ? 4
Lines ? 244
Branches ? 64
=======================================
Hits ? 225
Misses ? 19
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR corrects the typing for the fs.promises.readdir mock in the S3 uploader tests to accurately reflect the actual implementation's behavior.
Key Changes:
- Updated the Jest spy typing to explicitly specify the
string[]return type overload - Removed the incorrect
as unknown as fs.Dirent[]cast from the mock resolved value
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ec91fa9 to
f33ff7e
Compare
f33ff7e to
415bff5
Compare
Summary by Sourcery
Adjust S3 uploader tests to use more precise typings for mocked file system reads.
Enhancements:
Tests: