test_solution_0.1#1032
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This requires changes because a core requirement from the task description has not been met. You've done a great job setting up the test file and covering several valid scenarios, which is a solid start. However, the test suite is incomplete as it's missing the specific examples you were required to test.
Please add tests for the following cases to ensure full compliance:
splitInteger(8, 1)splitInteger(6, 2)splitInteger(17, 4)splitInteger(32, 6)
Once these required tests are added, your solution will be ready for approval. 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
| test('should add zeros if value < numberOfParts', () => { | ||
|
|
||
| expect(splitInteger(5, 7)).toEqual([0, 0, 1, 1, 1, 1, 1]); |
There was a problem hiding this comment.
Your current test cases are good, but the test suite is incomplete. The task description requires you to test a specific set of examples to ensure the function's correctness. Please add tests for the following cases: splitInteger(8, 1), splitInteger(6, 2), splitInteger(17, 4), and splitInteger(32, 6).
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent progress on adding the required test cases! You're very close to a complete solution. However, one key requirement is not yet met. The test for the specific examples from the description currently only checks if the output is sorted, but it needs to verify the exact result.
In src/splitInteger.test.js, please update your test.each block to include the expected array for each case and use toEqual to assert the result. For example, the test for splitInteger(17, 4) should verify that the output is exactly [4, 4, 4, 5].
Once you make that adjustment, this will be ready for approval!
✨ 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
| test.each([ | ||
| [8, 1], | ||
| [6, 2], | ||
| [17, 4], | ||
| [21, 4], | ||
| [32, 6], | ||
| [5, 7], | ||
| ])('splitInteger(%i, %i) should return an ascending ' | ||
| + 'sorted array (from lowest to highest)', | ||
| (value, parts) => { | ||
| const result = splitInteger(value, parts); | ||
|
|
||
| for (let i = 0; i < result.length - 1; i++) { | ||
| expect(result[i]).toBeLessThanOrEqual(result[i + 1]); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Great job adding the required examples to a test.each block! This is a good approach. However, this test currently only checks if the output is sorted. The requirement is to verify the exact result for each example (e.g., that splitInteger(17, 4) returns [4, 4, 4, 5]).
You can update this test.each to include the expected array for each case and use toEqual to assert the result.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on this iteration! I am approving your solution because you've done a fantastic job addressing the previous feedback. Adding tests to verify the exact output for the examples from the description was a key improvement and has made your test suite much more robust. Your tests are now comprehensive, covering all the requirements of the task. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.