Skip to content

Create tests22#930

Closed
Maryna4444-alt wants to merge 1 commit into
mate-academy:mainfrom
Maryna4444-alt:main
Closed

Create tests22#930
Maryna4444-alt wants to merge 1 commit into
mate-academy:mainfrom
Maryna4444-alt:main

Conversation

@Maryna4444-alt
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Thank you for your work! There is a critical issue: the implementation sorts the result before returning, but the expected output arrays in the tests have the larger numbers at the end, not the beginning. If the order of the output matters (as implied by the tests), sorting the result will produce incorrect outputs. Please update your implementation to return the result in the correct order—typically, the larger parts (those with the extra 1) should come first or last as required by the task or tests, and you should remove the sorting step if it is not required. Once this is fixed, your solution should meet all requirements! 👍


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread splitInteger.js
result[i]++;
}

return result.sort((a, b) => a - b);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorting the result here may violate the requirement if the task expects the larger parts (those with the extra 1) to come first. Please check the task description or checklist regarding the required order of the output. If order matters, remove the .sort((a, b) => a - b) and return result as is.

Comment thread splitInteger.test.js
Comment on lines +6 to +7
expect(splitInteger(17, 4)).toEqual([4, 4, 4, 5]);
expect(splitInteger(32, 6)).toEqual([5, 5, 5, 5, 6, 6]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expected output arrays have the larger numbers at the end, which may not match the output of the current implementation if it sorts the result. Please ensure that the implementation of splitInteger returns the parts in the correct order as expected by these tests.

Copy link
Copy Markdown

@Oleksii-Siviolov Oleksii-Siviolov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have the example of the tests in the file, which was in the task (we even cloned it on the Q&A session). Please do not remove anything from the existing file and just add your expects. Also, do not add anything extra (which you do not understand).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants