Skip to content

fix:solution 2#322

Open
mlabucki wants to merge 2 commits intomate-academy:masterfrom
mlabucki:develop_ML
Open

fix:solution 2#322
mlabucki wants to merge 2 commits intomate-academy:masterfrom
mlabucki:develop_ML

Conversation

@mlabucki
Copy link

No description provided.

Copy link

@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

Great work on this task! I am approving your solution as you've successfully written tests that cover all the core requirements for the restoreNames function.

To make your test suite even more robust in the future, here are a couple of suggestions you could consider:

  • Add a test case for an array containing multiple user objects, including a mix of users that need their names restored and those that don't.
  • Test edge cases, such as passing an empty array [] to the function, to ensure it handles them gracefully.

Also, I noticed a small thing: there's an unused import in src/restoreNames.test.js that can be removed for cleaner code. Excellent job overall!


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

Footnotes

  1. Rate AI review example

@@ -1,10 +1,48 @@
'use strict';

const { ExportConflictException } = require('@aws-sdk/client-dynamodb');

Choose a reason for hiding this comment

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

This import seems to be unused in this test file. It's good practice to remove any unnecessary dependencies or leftover code.

expect(users[0].firstName).toBe('Jack')
});

// write tests here

Choose a reason for hiding this comment

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

The current tests are good for individual cases. To make the test suite more comprehensive, consider adding a test for an array containing multiple user objects, some that need fixing and some that don't. Testing edge cases, like an empty array, would also be a valuable addition.

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