-
Notifications
You must be signed in to change notification settings - Fork 4k
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
tests(shorthand): allow non-strict assert #1845
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1845 +/- ##
=========================================
Coverage ? 99.75%
=========================================
Files ? 145
Lines ? 2477
Branches ? 0
=========================================
Hits ? 2471
Misses ? 6
Partials ? 0
Continue to review full report at Codecov.
|
@@ -63,6 +63,7 @@ describe('Input', () => { | |||
common.implementsHTMLInputProp(Input, { | |||
alwaysPresent: true, | |||
shorthandDefaultProps: { type: 'text' }, | |||
strictAssert: 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.
How about assertExactMatch
or similar? It is not clear what is being strictly asserted.
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.
Yep, I like your suggestion, will change 👍
…b.com/Semantic-Org/Semantic-UI-React into tests/shorthand-strict # Conflicts: # test/specs/commonTests/implementsShorthandProp.js
ef1b09a
to
7c3f281
Compare
…b.com/Semantic-Org/Semantic-UI-React into tests/shorthand-strict # Conflicts: # test/specs/commonTests/implementsShorthandProp.js
@levithomason I've changed an option name 👍 |
Released in |
Why?
Extracted from #1795.
contains
is too strict, we should allow non-strict assert in these cases, useless conditions should gone.