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

Refactor email test with table-driven test and add more cases #1281

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daisy1754
Copy link

Fixes Or Enhances

Added a few test cases around domains that contain number. One thing I realized was validator currently reject address like example.com1 which I believe is allowed per RFC but practically doesn't exist in IANA list. I wasn't sure if it's bug or feature, but at least having test case make it obvious for future reader.

Also refactored TestEmail to use Table Driven Test to avoid code duplication and make test result obvious. Before this, test failed with --- FAIL: TestEmail (0.00s), now it will be like

    --- PASS: TestEmail/Email:_"test_test"@email.com (0.00s)
    --- PASS: TestEmail/Email:_"@email.com (0.00s)
    --- FAIL: TestEmail/Email:[email protected] (0.00s)

Make sure that you've checked the boxes below before you submit PR:

  • Tests exist or have been written that cover this particular change.

@go-playground/validator-maintainers

@daisy1754 daisy1754 requested a review from a team as a code owner June 16, 2024 14:19
@coveralls
Copy link

Coverage Status

coverage: 74.291%. remained the same
when pulling ea408f3 on daisy1754:kazuki/more_email_test
into a947377 on go-playground:master.

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