Wrote pytests for 'split_integer' function.#2166
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Two core functional requirements from the task description are completely missing from the test suite: (1) verifying the result has exactly number_of_parts elements, and (2) verifying the max-min difference is ≤ 1. These are explicit requirements that must be tested. Additionally, [CHECKLIST ITEM #1] is violated in multiple tests where split_integer() is called multiple times instead of storing the result once in a variable. [CHECKLIST ITEM #2] is also violated where assertions like 0 in result don't compare against an expected result. Please add the two missing tests for core requirements and refactor existing tests to call the function once and assert against expected values.
✨ 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
| ) -> None: | ||
| assert split_integer( | ||
| value, | ||
| number_of_parts | ||
| )[0] == split_integer( | ||
| value, | ||
| number_of_parts | ||
| )[-1] |
There was a problem hiding this comment.
Violates checklist #1: This test calls split_integer twice instead of saving the result once. The good pattern is assert some_function() == expected_value without creating intermediate variables. Consider: result = split_integer(value, number_of_parts) and then assert against result.
| ) -> None: | ||
| assert split_integer( | ||
| value, | ||
| number_of_parts | ||
| ) == sorted(split_integer( | ||
| value, | ||
| number_of_parts | ||
| )) |
There was a problem hiding this comment.
Violates checklist #1: This test calls split_integer three times instead of calling it once. Save the result in a variable and assert it equals the sorted version.
| ) -> None: | ||
| assert 0 in split_integer(value, number_of_parts) |
There was a problem hiding this comment.
Violates checklist #2: The assertion 0 in split_integer(...) doesn't compare against an expected result. It should compare directly like assert split_integer(20, 21) == [0] * 21 or similar expected output.
| import pytest | ||
|
|
||
| from app.split_integer import split_integer | ||
|
|
||
|
|
||
| def test_sum_of_the_parts_should_be_equal_to_value() -> None: | ||
| pass | ||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (1, 1), | ||
| (100, 19), | ||
| (12, 11) | ||
|
|
||
| ] | ||
| ) | ||
| def test_sum_of_the_parts_should_be_equal_to_value( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert sum(split_integer(value, number_of_parts)) == value | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (20, 10), | ||
| (60, 15), | ||
| (1000, 50) | ||
|
|
||
| ] | ||
| ) | ||
| def test_should_split_into_equal_parts_when_value_divisible_by_parts( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert split_integer( | ||
| value, | ||
| number_of_parts | ||
| )[0] == split_integer( | ||
| value, | ||
| number_of_parts | ||
| )[-1] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (17, 1), | ||
| (9, 1), | ||
| (1000, 1) | ||
|
|
||
| def test_should_split_into_equal_parts_when_value_divisible_by_parts() -> None: | ||
| pass | ||
| ] | ||
| ) | ||
| def test_should_return_part_equals_to_value_when_split_into_one_part( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert split_integer(value, number_of_parts) == [value] | ||
|
|
||
|
|
||
| def test_should_return_part_equals_to_value_when_split_into_one_part() -> None: | ||
| pass | ||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (20, 9), | ||
| (4, 3), | ||
| (1000, 54) | ||
|
|
||
| ] | ||
| ) | ||
| def test_parts_should_be_sorted_when_they_are_not_equal( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert split_integer( | ||
| value, | ||
| number_of_parts | ||
| ) == sorted(split_integer( | ||
| value, | ||
| number_of_parts | ||
| )) | ||
|
|
||
| def test_parts_should_be_sorted_when_they_are_not_equal() -> None: | ||
| pass | ||
|
|
||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (20, 21), | ||
| (0, 10), | ||
| (10, 19) | ||
|
|
||
| def test_should_add_zeros_when_value_is_less_than_number_of_parts() -> None: | ||
| pass | ||
| ] | ||
| ) | ||
| def test_should_add_zeros_when_value_is_less_than_number_of_parts( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert 0 in split_integer(value, number_of_parts) |
There was a problem hiding this comment.
Missing test: The task requires 'returns an array containing exactly number_of_parts integer elements' but no test verifies this core requirement. Add a test that checks len(split_integer(value, parts)) == parts.
| import pytest | ||
|
|
||
| from app.split_integer import split_integer | ||
|
|
||
|
|
||
| def test_sum_of_the_parts_should_be_equal_to_value() -> None: | ||
| pass | ||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (1, 1), | ||
| (100, 19), | ||
| (12, 11) | ||
|
|
||
| ] | ||
| ) | ||
| def test_sum_of_the_parts_should_be_equal_to_value( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert sum(split_integer(value, number_of_parts)) == value | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (20, 10), | ||
| (60, 15), | ||
| (1000, 50) | ||
|
|
||
| ] | ||
| ) | ||
| def test_should_split_into_equal_parts_when_value_divisible_by_parts( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert split_integer( | ||
| value, | ||
| number_of_parts | ||
| )[0] == split_integer( | ||
| value, | ||
| number_of_parts | ||
| )[-1] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (17, 1), | ||
| (9, 1), | ||
| (1000, 1) | ||
|
|
||
| def test_should_split_into_equal_parts_when_value_divisible_by_parts() -> None: | ||
| pass | ||
| ] | ||
| ) | ||
| def test_should_return_part_equals_to_value_when_split_into_one_part( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert split_integer(value, number_of_parts) == [value] | ||
|
|
||
|
|
||
| def test_should_return_part_equals_to_value_when_split_into_one_part() -> None: | ||
| pass | ||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (20, 9), | ||
| (4, 3), | ||
| (1000, 54) | ||
|
|
||
| ] | ||
| ) | ||
| def test_parts_should_be_sorted_when_they_are_not_equal( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert split_integer( | ||
| value, | ||
| number_of_parts | ||
| ) == sorted(split_integer( | ||
| value, | ||
| number_of_parts | ||
| )) | ||
|
|
||
| def test_parts_should_be_sorted_when_they_are_not_equal() -> None: | ||
| pass | ||
|
|
||
| @pytest.mark.parametrize( | ||
| "value, number_of_parts", | ||
| [ | ||
| (20, 21), | ||
| (0, 10), | ||
| (10, 19) | ||
|
|
||
| def test_should_add_zeros_when_value_is_less_than_number_of_parts() -> None: | ||
| pass | ||
| ] | ||
| ) | ||
| def test_should_add_zeros_when_value_is_less_than_number_of_parts( | ||
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| assert 0 in split_integer(value, number_of_parts) |
There was a problem hiding this comment.
Missing test: The task requires 'the difference between the max and min number in the array should be <= 1' but this critical requirement is not tested. Add a test that verifies max(result) - min(result) <= 1.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
All core functional requirements are met and tests pass. The test suite covers exact element count and max-min difference checks. There is one minor style issue remaining: in test_difference_between_parts_should_be_less_or_equal_than_1, the result is stored in an unnecessary result variable before asserting. Per [CHECKLIST ITEM #1], you can write the assertion directly without the intermediate variable, like assert max(split_integer(value, parts)) - min(split_integer(value, parts)) <= 1. This is not blocking your progress, but cleaning it up would follow the checklist guidelines.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| value: int, | ||
| number_of_parts: int | ||
| ) -> None: | ||
| result = split_integer(value, number_of_parts) |
There was a problem hiding this comment.
This violates checklist item #1: Don't create unnecessary variables for simple tests. The assertion can be written directly without storing the result in a variable.
2 participants
No description provided.