-
Notifications
You must be signed in to change notification settings - Fork 44
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 url validation to rules step #1180
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1180 +/- ##
=======================================
Coverage 44.09% 44.10%
=======================================
Files 177 177
Lines 4499 4501 +2
Branches 1007 1007
=======================================
+ Hits 1984 1985 +1
- Misses 2504 2505 +1
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Signed-off-by: ibolton336 <ibolton@redhat.com>
const brokenURLExample = "github.com/ibolton336/tackle-testapp.git"; | ||
const brokenURLvalidationResult = gitUrlRegex.test(brokenURLExample); | ||
expect(brokenURLvalidationResult).toBe(false); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest adding a few more strings to the tests, and have an it()
section for gitUrlRegex
and standardURLRegex
.
A few url string suggestions:
""
" http://www.foo.bar"
" http://www.foo.bar "
"www.foo.bar"
"foo.bar"
"www.foo.b"
"foo.ba"
Having a separate it()
just for a double test would be good as well:
it("URL should match the same multiple times in a row", () => {
const url = "https://github.com/ibolton336/tackle-testapp.git";
expect(standardURLRegex.test(url).toBe(true);
expect(standardURLRegex.test(url).toBe(true);
expect(standardURLRegex.test(url).toBe(true);
expect(standardURLRegex.test(url).toBe(true);
expect(standardURLRegex.test(url).toBe(true);
});
And that test should fail if you put the /g
back on the pattern...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jeez, I would not have expected a regex test
to have state like that
Signed-off-by: ibolton336 <ibolton@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
The tests look good so they capture all of the troubles that have been dealt with. 👍
Resolves https://issues.redhat.com/browse/MTA-985 --------- Signed-off-by: ibolton336 <ibolton@redhat.com>
) Resolves https://issues.redhat.com/browse/MTA-985 Signed-off-by: ibolton336 <ibolton@redhat.com>
Resolves https://issues.redhat.com/browse/MTA-985