test: add unit tests for split_integer function#2151
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because the implementation and tests correctly cover the required behaviors of split_integer and there are no functional blockers. Your tests verify key examples like split_integer(8, 1), split_integer(6, 2), split_integer(17, 4), and the distribution logic for larger values such as 32 with multiple parts, which aligns with the task requirements.
The main issue is stylistic: in app/test_split_integer.py, each test creates an intermediate result variable (for example around lines where you do result = split_integer(32, 6) and then immediately assert on result), which goes against [CHECKLIST ITEM #1] about avoiding unnecessary variables in simple tests. A cleaner pattern would be:
assert split_integer(32, 6) == [5, 5, 5, 5, 6, 6]assert split_integer(6, 2) == [3, 3]assert split_integer(8, 1) == [8]
These are non-blocking style issues and do not prevent task completion, but you should adopt this pattern going forward and also ensure there are no leftover debug prints or helpers in other files as required by the checklist. Overall, your functional coverage is solid and this is a good implementation; just refine the test style in future exercises.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
|
|
||
| def test_sum_of_the_parts_should_be_equal_to_value() -> None: | ||
| pass | ||
| result = split_integer(32, 6) |
There was a problem hiding this comment.
This violates checklist item #1: Don't create unnecessary variables for simple tests because you assign result = split_integer(32, 6) and then only use it in the assert. For simple equality or property checks, compare split_integer(32, 6) directly in the assert.
|
|
||
| def test_should_split_into_equal_parts_when_value_divisible_by_parts() -> None: | ||
| pass | ||
| result = split_integer(6, 2) |
There was a problem hiding this comment.
This test also violates checklist item #1: Don't create unnecessary variables for simple tests by introducing result when it’s only used once in the following assert. Consider writing assert split_integer(6, 2) == [3, 3] directly.
|
|
||
| def test_should_return_part_equals_to_value_when_split_into_one_part() -> None: | ||
| pass | ||
| result = split_integer(8, 1) |
There was a problem hiding this comment.
Here again you introduce an unnecessary result variable, which matches the "Bad example" pattern in checklist item #1. The test can be simplified to assert split_integer(8, 1) == [8].
|
|
||
| def test_parts_should_be_sorted_when_they_are_not_equal() -> None: | ||
| pass | ||
| result = split_integer(17, 4) |
There was a problem hiding this comment.
This line follows the same unnecessary-variable pattern as the checklist’s "Bad example". Checklist item #1 asks to avoid such temporary variables in simple tests; you can assert directly on split_integer(17, 4).
|
|
||
| def test_should_add_zeros_when_value_is_less_than_number_of_parts() -> None: | ||
| pass | ||
| result = split_integer(2, 5) |
There was a problem hiding this comment.
This test also uses an unnecessary result variable, violating checklist item #1. For simple tests, avoid this pattern and assert directly on the function call.
2 participants
No description provided.