test: add tests for reduce method#94
test: add tests for reduce method#94mykhailokurochkin wants to merge 3 commits intomate-academy:masterfrom
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This is a great start, and your tests for reduce with an initial value are well-written. However, this requires changes because a major part of the reduce functionality is currently untested, which is a critical requirement.
Specifically, your test suite is missing the entire scenario where no initialValue is provided. Please add tests to cover the following cases:
- When
reduceis called without an initial value, it should use the first element as the accumulator and start iterating from the second element. - Calling
reduceon an empty array without an initial value should throw aTypeError. - The test for callback arguments in
src/reduce.test.jsshould be more thorough. As noted in the comments, consider usingtoHaveBeenNthCalledWithto verify the arguments for more than just the first call.
Adding these tests will ensure your implementation is robust and fully compliant with the reduce specification.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
src/reduce.test.js
Outdated
| const cb = jest.fn(); | ||
|
|
||
| arr.reduce2(cb, 0); | ||
| expect(cb).toHaveBeenCalledWith(0, 0, 0, arr); |
There was a problem hiding this comment.
This assertion is a good start, but it only verifies the arguments for the very first call to the callback. It doesn't confirm that subsequent calls receive the correctly updated accumulator, currentValue, and currentIndex. Consider using toHaveBeenNthCalledWith to test a few more calls to make your test more thorough.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this update! You've successfully addressed almost all the feedback, and the test suite is very robust. I am requesting one final change before approving.
The test suite is missing a test for a specific edge case: calling reduce on an array with a single element when no initialValue is provided. According to the MDN specification, the method should return the single element without invoking the callback.
Please add a test case to src/reduce.test.js to cover this, similar to the following:
it('should return the element without calling the callback for a single-element array', () => {
const cb = jest.fn();
const singleElementArray = [42];
expect(singleElementArray.reduce2(cb)).toBe(42);
expect(cb).not.toHaveBeenCalled();
});Once this is added, your work will be complete. You're very close!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| it('throws TypeError if array and initial value are empty', () => { | ||
| expect(() => [].reduce2(sumCb)).toThrow(TypeError); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This is great! To make the test suite fully comprehensive, please add one more test case to cover an important edge case from the requirements: calling reduce on an array with a single element and no initialValue.
According to the MDN specification, in this situation, the single element should be returned directly, and the callback function should not be executed. A test for this would look something like this:
it('should return the element without calling the callback for a single-element array', () => {
const cb = jest.fn();
const singleElementArray = [42];
expect(singleElementArray.reduce2(cb)).toBe(42);
expect(cb).not.toHaveBeenCalled();
});
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent progress on this task! I am approving your submission as an educational override to keep you moving forward, but please take a moment to address one small point. You did a great job adding the test case for a single-element array in src/reduce.test.js and correctly asserted that the callback is not invoked. However, the test is missing a key assertion: it also needs to verify that the method returns the single element itself. To complete this, please add an assertion to check that the result is 1. This will make your test suite fully cover this important edge case. Keep up the fantastic work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| [1].reduce2(cb); | ||
|
|
||
| expect(cb).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
Great job adding this test case for a single-element array without an initial value. You've correctly checked that the callback is not invoked. To fully cover this scenario, please also add an assertion to verify that the method returns the single element itself (in this case, 1).
No description provided.