🧪 [testing improvement] Add unit tests for usePdfStore toBlob helper#125
🧪 [testing improvement] Add unit tests for usePdfStore toBlob helper#125
Conversation
Exported the internal `toBlob` helper function in `src/modules/pdf-forge/store/usePdfStore.ts` and added comprehensive unit tests to `usePdfStore.test.ts`.
Covered scenarios:
- Normal conversion of `Uint8Array` to `Blob`
- Handling of empty `Uint8Array`
- Assertions on `Blob` instance, type ('application/pdf'), and expected size.
Co-authored-by: Cukurikik <266119688+Cukurikik@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/modules/pdf-forge/store/usePdfStore.ts (1)
33-35: Usingdata.buffercan include unintended bytes whenUint8Arrayis a subarray view.
Uint8Array.bufferreturns the entire underlyingArrayBuffer, which may be larger than the view if theUint8Arraywas created with an offset or as a subarray. Passdatadirectly to theBlobconstructor instead—it acceptsTypedArraynatively.Proposed fix
export function toBlob(data: Uint8Array): Blob { - return new Blob([data.buffer as ArrayBuffer], { type: 'application/pdf' }); + return new Blob([data], { type: 'application/pdf' }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/pdf-forge/store/usePdfStore.ts` around lines 33 - 35, The toBlob function uses data.buffer which can include extra bytes when the Uint8Array is a subarray; change the Blob construction to pass the Uint8Array itself (the data variable) instead of data.buffer so the Blob contains only the view's bytes and keep the MIME type 'application/pdf'; update the implementation in the toBlob function accordingly.src/modules/pdf-forge/store/usePdfStore.test.ts (1)
55-73: LGTM — consider adding a subarray edge-case test.The current tests cover basic conversion and empty input well. To guard against the
data.bufferissue noted in the implementation, consider adding a test with aUint8Arraysubarray view:Optional additional test
it('should handle Uint8Array subarray views correctly', () => { const buffer = new ArrayBuffer(10); const fullView = new Uint8Array(buffer); fullView.set([0, 0, 1, 2, 3, 0, 0, 0, 0, 0]); const subarray = fullView.subarray(2, 5); // [1, 2, 3] const blob = toBlob(subarray); expect(blob.size).toBe(3); // Should be 3, not 10 expect(blob.type).toBe('application/pdf'); });This test would fail with the current implementation (which uses
data.buffer) and pass once fixed to usedatadirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/pdf-forge/store/usePdfStore.test.ts` around lines 55 - 73, The test needs to cover the Uint8Array subarray edge case and the implementation of toBlob must be updated to avoid using the whole data.buffer; modify tests to add a case that passes a subarray view (as suggested) and update the toBlob implementation (function toBlob) to construct the Blob from the actual Uint8Array slice/bytes (use the Uint8Array itself or new Uint8Array(data.buffer, data.byteOffset, data.byteLength)) so the Blob size equals data.byteLength rather than the full ArrayBuffer length.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/modules/pdf-forge/store/usePdfStore.test.ts`:
- Around line 55-73: The test needs to cover the Uint8Array subarray edge case
and the implementation of toBlob must be updated to avoid using the whole
data.buffer; modify tests to add a case that passes a subarray view (as
suggested) and update the toBlob implementation (function toBlob) to construct
the Blob from the actual Uint8Array slice/bytes (use the Uint8Array itself or
new Uint8Array(data.buffer, data.byteOffset, data.byteLength)) so the Blob size
equals data.byteLength rather than the full ArrayBuffer length.
In `@src/modules/pdf-forge/store/usePdfStore.ts`:
- Around line 33-35: The toBlob function uses data.buffer which can include
extra bytes when the Uint8Array is a subarray; change the Blob construction to
pass the Uint8Array itself (the data variable) instead of data.buffer so the
Blob contains only the view's bytes and keep the MIME type 'application/pdf';
update the implementation in the toBlob function accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d82ecda-f903-4995-a4dd-1c8866a9005f
📒 Files selected for processing (2)
src/modules/pdf-forge/store/usePdfStore.test.tssrc/modules/pdf-forge/store/usePdfStore.ts
🎯 What: The testing gap in
src/modules/pdf-forge/store/usePdfStore.tsregarding thetoBlobpure function was addressed.📊 Coverage: Added tests to cover the happy path (correct conversion, correct blob type, correct size) and an edge condition (empty Uint8Array) in
usePdfStore.test.ts.✨ Result: Improved test coverage and reliability for the PDF store utility functions. The
toBlobfunction was exported to be testable.PR created automatically by Jules for task 4187336579000509184 started by @Cukurikik
Summary by CodeRabbit
Refactor
Tests