add allowFunctions as an allow-list for require-expect function calls
#233
Conversation
|
|
||
| If you are using the Check Type `except-simple` whenever you pass `assert` into | ||
| another function you will cause an error. If you have some functions that you | ||
| are passing `assert` to that don't add to the number of asserts (and maybe just |
There was a problem hiding this comment.
I think the part in parentheses is probably unnecessary. Would you mind removing it?
| "type": "object", | ||
| "properties": { | ||
| "allowFunctions": { | ||
| "type": "array" |
There was a problem hiding this comment.
Can we add to this to require that all items in the array should be strings? Also, can we only allow this option if the previous value is except-simple?
| if (!currentTest.assertName) { | ||
| return false; | ||
| } | ||
|
|
||
| if (allowedFunctions.includes(node.callee.name)) { |
There was a problem hiding this comment.
This will only work with identifiers. Do we want to support property accesses (for example, myLib.preprocess(assert)?
| "test('name', function(assert) { myAssertion(null, assert, null); });" | ||
| ].join(returnAndIndent), | ||
| options: ["except-simple", { allowFunctions: ["myAssertion"] }], | ||
| errors: [exceptSimpleErrorMessage("assert.expect")] |
There was a problem hiding this comment.
Shouldn't this code pass? Please remove the errors property.
| Example: | ||
|
|
||
| ```js | ||
| rules: { |
There was a problem hiding this comment.
In order for eslint-plugin-markdown to pass, please surround this object with curly braces and remove the dangling comma at the end. Thanks!
| Here is an example: | ||
|
|
||
| ```js | ||
| rules: { |
There was a problem hiding this comment.
In order for eslint-plugin-markdown to pass, please surround this object with curly braces and remove the dangling comma at the end. Thanks!
|
One note: we are planning to change the default config of require-expect rule to |
|
Hi @mansona, I apologize for letting this sit so long. Let me know in the next week or so if you are still interested in finishing this up; if not, I'll go ahead and close this. In either case, I appreciate the work so far and welcome future contributions! |
|
Closing this due to inactivity. Feel free to resubmit this if desired (original PR creator or anyone else). Thanks for contributing! |
Hey folks 👋
I was following the documentation for
@percy/emberand they have an example that allows you to pass theassertargument into apercySnapshot()call to automatically interpret the test name and module name https://github.com/percy/percy-ember#qunitBut if you have
require-expectconfigured with the default Check Typeexcept-simplethis will fail because you shouldn't passassertto other functions.I was thinking about it and I wanted to make use of this functionality in our application but I didn't want to disable the
require-expectrule (or even change the current config of it). I considered doing an ignore for each case that I wanted to use this functionality but this just didn't seem right. I thought in the end the Best Solution™️ would be to have the ability to configure which functions I should allow you to passassertinto.And this PR adds the ability to add a new config option to add an allow-list for passing
assertinto functions 🎉Let me know if you have any questions or if you would like me to change or improve this PR in any way 👍