Skip to content

Solution#249

Open
Nazarii84 wants to merge 1 commit intomate-academy:masterfrom
Nazarii84:develop
Open

Solution#249
Nazarii84 wants to merge 1 commit intomate-academy:masterfrom
Nazarii84:develop

Conversation

@Nazarii84
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

Excellent work on the tests for the ifElse function! I am approving your solution as it correctly verifies the core conditional logic and meets the main requirements.

For future improvements, consider making your assertions more specific. The task description mentions that the callbacks should be called with no arguments. While your tests correctly check if the functions are called using toHaveBeenCalled(), you could use toHaveBeenCalledWith() to also verify they were called with no arguments. This would make your tests even more robust and fully aligned with all requirements.

This is a minor point, and your current implementation is solid. Keep up the great work!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment on lines +14 to +15
expect(first).toHaveBeenCalled();
expect(second).not.toHaveBeenCalled();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These assertions are correct, but they could be more specific. The task description mentions that condition should be called with no arguments (and it's implied for first as well).

To fully test this, you could also:

  1. Make condition a jest.fn() to check if it has been called.
  2. Use toHaveBeenCalledWith() to verify that both condition and first are called with no arguments.

Comment on lines +26 to +27
expect(second).toHaveBeenCalled();
expect(first).not.toHaveBeenCalled();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar to the previous test, you can make these assertions more precise. The requirements explicitly state that condition and second should be called with no arguments.

Consider making condition a mock function and using toHaveBeenCalledWith() for both condition and second to ensure they are called correctly.

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.

2 participants