Repository navigation
added tests - #199
added tests#199yahohulia wants to merge 2 commits into
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the current tests enforce behavior that is not required by the task and also miss a mandated example from the specification. In src/arrayReverse.test.js, you add tests for empty arrays, empty strings, mixes of empty and non-empty strings, and in-place mutation; these introduce extra constraints that go beyond the described requirements and can incorrectly fail valid implementations [CHECKLIST ITEM #3]. Please remove or relax those tests so they only check what the task explicitly specifies (reversing strings and their order), rather than assuming how empty inputs or mutation should behave. Additionally, the task explicitly gives an example arrayReverse(['Hell0']) === ['0lleH'], which is currently not tested; add a dedicated test case that uses exactly this input and expected output [CHECKLIST ITEM #5]. As an improvement (not strictly blocking), consider adding a focused test that demonstrates the described length/character transfer rule and a case including digits/special symbols to better reflect the allowed character set [CHECKLIST ITEMS #2 and #4]. Once you adjust the tests to align strictly with the documented behavior and include the required example, your test suite will properly guide and validate student solutions. Overall, your existing tests for ['Mate','Academy'] and ['I','am','a','student!'] are well written and correctly use Jest, so you are very close—just refine the scope and examples to match the spec.
✨ 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(`should return an empty array if the original array is empty`, () => { |
There was a problem hiding this comment.
This test enforces behavior (returning an empty array for []) that is not specified in the task description; per the checklist we should avoid inventing extra required behavior beyond the given examples.
| expect(arrayReverse([])).toEqual([]); | ||
| }); | ||
|
|
||
| it(`should handle an array that consists of an empty string`, () => { |
There was a problem hiding this comment.
Handling of an empty string as input is not mentioned in the requirements; consider removing this test or only testing behaviors that are explicitly required.
| expect(arrayReverse([''])).toEqual(['']); | ||
| }); | ||
|
|
||
| it(`should correctly handle a mix of empty and non-empty strings`, () => { |
There was a problem hiding this comment.
This test introduces a requirement for how to treat mixes of empty and non-empty strings, which is outside the scope of the described task; it can over-constrain valid implementations.
| expect(arrayReverse(['a', '', 'bc', ''])).toEqual(['c', '', 'ba', '']); | ||
| }); | ||
|
|
||
| it(`should mutate the original array (in-place modification)`, () => { |
There was a problem hiding this comment.
This test asserts in-place mutation, but the description does not require arrayReverse to modify its input array. Enforcing mutation can conflict with valid non-mutating implementations, so this should be removed unless the spec explicitly requires it.
| it(`should reverse a single word`, () => { | ||
| expect(arrayReverse(['Hello'])).toEqual(['olleH']); | ||
| }); |
There was a problem hiding this comment.
The task explicitly provides an example arrayReverse(['Hell0']) === ['0lleH'] that is not currently tested; consider adding a dedicated test case using that exact input and output.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved because the test file now fully matches the task requirements and focuses only on the specified behavior of arrayReverse. The tests correctly verify that arrayReverse is a function and use Jest expectations as required. You included all three mandated examples: ['Hell0'] -> ['0lleH'], ['Mate','Academy'] -> ['ymed','acAetaM'], and ['I','am','a','student!'] -> ['!','tn','e','dutsamaI'], which together validate reversing string order, redistributing characters, and handling letters, digits, and punctuation. You also removed earlier over-constraining tests (like empty input and mutation checks), so the suite no longer enforces behavior that wasn’t specified in the task. There are no critical or blocking issues remaining, and the file is clear, focused, and idiomatic. Nice work aligning your tests precisely with the problem description and keeping them minimal yet complete.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.