Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CreateAddressBookItemDto schema tests #2151

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hectorgomezv
Copy link
Member

Changes

  • Adds missing schema tests for CreateAddressBookItemDto.

@hectorgomezv hectorgomezv self-assigned this Nov 22, 2024
@hectorgomezv hectorgomezv requested a review from a team as a code owner November 22, 2024 14:40
Copy link
Member

@iamacook iamacook left a comment

Choose a reason for hiding this comment

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

The address tests look good, but I think we should move most of the name tests to a separate spec that only tests AddressBookItemNameSchema. What do you think?

I suggest we adjust the error messages of it as well:

  • "Address Books items..." -> "Address book entry names...".
  • "...and can contain..." -> "...and can only contain...".

@jmealy
Copy link
Contributor

jmealy commented Dec 23, 2024

The address tests look good, but I think we should move most of the name tests to a separate spec that only tests AddressBookItemNameSchema. What do you think?

I suggest we adjust the error messages of it as well:

  • "Address Books items..." -> "Address book entry names...".
  • "...and can contain..." -> "...and can only contain...".

Looks like tests for the name schema already exist in address-book-item-name.schema.spec.ts. They cover the same cases as the name tests here.

Changed the error message as suggested and fixed a typo: 9797af0

@jmealy jmealy requested a review from PooyaRaki December 23, 2024 10:22
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.

3 participants