[backend] Domain observable checker is incorrect (#7620) #8768
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8768 +/- ##
=======================================
Coverage 66.30% 66.30%
=======================================
Files 597 597
Lines 61033 61033
Branches 6275 6279 +4
=======================================
Hits 40468 40468
Misses 20565 20565 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Tested ok with corrects and incorrects domains and subdomains.
the only case that seems to be not covered by test is a very long subdomain:
"very0long0subdomain0that0exceeds0the0sixty0three0character0limit.mydomain.com"
subdomain are limited to 63 characters to be valid
| @@ -12,6 +12,8 @@ describe('Regex Pattern Tests', () => { | |||
| expect('example.com').toMatch(domainChecker); | |||
| expect('sub.example.co.uk').toMatch(domainChecker); | |||
| expect('løveskateboards.com').toMatch(domainChecker); | |||
| expect('test._mysubdomain.mydomain.com').toMatch(domainChecker); | |||
| expect('test_mysubdomain.domain.io').toMatch(domainChecker); | |||
| expect('invalid_domain.12_3').not.toMatch(domainChecker); | |||
There was a problem hiding this comment.
maybe add a test that it's not working if there is a dot in the main name and no dots (exemple: invalid_domain)
|
I just suggested to add a little test case. Since everything is ok, I'm gonna approve the PR so you can merge it after adding the test. |
SarahBocognano
force-pushed
the
issue/7620
branch
from
a4f445a to
baf58e8
Compare
Proposed changes
Related issues