Test suite for email format is very weak

[bvisness]

Today we started using a JSON schema library in production which passes all tests in this repo, including the optional ones. It blew up immediately on valid customer email addresses which contained numbers in the domain.

Wondering how this could have happened, I looked up these tests, only to find that you have provided exactly one test of a valid email address:

{
    "description": "a valid e-mail address",
    "data": "[email protected]",
    "valid": true
}

Suffice it to say that this is a very poor test of the email format.

I know that the email address spec is very...esoteric. But I think you should include a few extra test cases that will hopefully break naive implementations like the one I was using. Ideally you would include some email addresses with digits and other allowed symbols in the address part, and possibly some email addresses with quoted parts. You may also wish to add some tests with unconventional hostnames, like the ones I added in #253.

[Julian]

This essentially harkens back to the fact that I wrote these, and I don't recommend actually using the email format, and my implementation implements it as "anything with @ in it".

In 2019, and especially now that the spec is active again and has been for ~2 years again (hooray!) I've still always meant to bring that back up upstream in the spec repo ("let's remove the email format plz"), but yeah that's no excuse really for this suite anymore :), since it's not just used by me, not for a very long time.

So, especially if your tests are designed to ensure emails are accepted -- I'm all for those (I'll start to get a bit more fussy if you try to define invalid emails, but even then of course my representation as maintainer [or one of] for this suite isn't hopefully clouded by my desire as implementer of my library to not encourage bad habits).

So, long story short, sounds good to me!

[bvisness]

Given what I've been learning about the state of format in JSON schema, I think it makes sense to focus only on valid emails. Better to have false positives than false negatives in this case.

I've updated #253 to add lots more examples of valid emails. With any luck, this will convince everyone to just look for an "@" and leave it at that. 😛

[Julian]

The PR here was merged, so gonna close this, but more tests are welcome if someone feels there's still a gap.