-
Notifications
You must be signed in to change notification settings - Fork 21
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
Limitation: no-assert-ok does not working for nested function #199
Comments
This is more of a known limitation at this point. I think we could enhance the rule to look for functions defined in the same file, and explicitly called by test functions. However, we would not be able to do this as easily with functions imported from a different file, and I'm not sure we should. PR would be welcome. We would need to use proper variable analysis (probably via the eslint-utils package) to trace as accurately as we can. We would need to make sure the assert variable name can be locally renamed and still caught. In other words, we need to add a unit test like this: QUnit.test("test", function (assert) {
assertWrapper(assert);
});
function assertWrapper(foo) {
foo.ok(true); // should be detected
} I would also accept a docs PR indicating the current known limitation (assertion must be in the test itself). |
rule enabled in
.eslintrc.js
:but the Rule ignores if
assert.ok
is used in shared function:For example:
https://github.com/Mifrill/demo-eslint-plugin-qunit-no-assert-ok-ignore-nested-function/blob/4053e43237290754a88939e49e27732feeba24d3/tests/acceptance/index-test.js#L1-L22
Lint passed:
https://github.com/Mifrill/demo-eslint-plugin-qunit-no-assert-ok-ignore-nested-function/runs/3266269169
Rule:
https://github.com/platinumazure/eslint-plugin-qunit/blob/master/docs/rules/no-assert-ok.md
Repo with bug reproduce:
https://github.com/Mifrill/demo-eslint-plugin-qunit-no-assert-ok-ignore-nested-function
Expected
lint:js
failed:Mifrill/demo-eslint-plugin-qunit-no-assert-ok-ignore-nested-function#1
The text was updated successfully, but these errors were encountered: