Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Ensure boolean assertions are not missed by several rules #172

Merged
merged 5 commits into from
Apr 21, 2021
Merged

Ensure boolean assertions are not missed by several rules #172

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
374cba0
fix: boolean assertions were missed by some rules
bmish Apr 10, 2021
b4f5f84
Update tests/lib/rules/assert-args.js
bmish Apr 21, 2021
15b6dc6
Update tests/lib/rules/assert-args.js
bmish Apr 21, 2021
ada4f6e
Update tests/lib/rules/assert-args.js
bmish Apr 21, 2021
3216960
Update tests/lib/rules/assert-args.js
bmish Apr 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ const ASSERTION_METADATA = {
allowedArities: [2],
compareActualFirst: true
},
false: {
allowedArities: [1]
},
notDeepEqual: {
allowedArities: [2],
compareActualFirst: true
Expand Down Expand Up @@ -56,6 +59,9 @@ const ASSERTION_METADATA = {
},
throws: {
allowedArities: [1, 2]
},
true: {
allowedArities: [1]
}
};

Expand Down
52 changes: 52 additions & 0 deletions tests/lib/rules/assert-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ ruleTester.run("assert-args", rule, {
wrap("assert.equal(result, expected, 'Message');"),
wrap("assert.equal(obj[key], expected, key + ' value is true');"),

// false
wrap("false(result);"),
bmish marked this conversation as resolved.
Show resolved Hide resolved
wrap("false(result, 'Message');"),
bmish marked this conversation as resolved.
Show resolved Hide resolved

// strictEqual
wrap("strictEqual(result, expected);"),
wrap("strictEqual(result, expected, 'Message');"),
Expand Down Expand Up @@ -98,6 +102,10 @@ ruleTester.run("assert-args", rule, {
wrap("assert.throws(function () {}, 'Error', 'Message');"),
wrap("assert.throws(function () {}, TypeError, expectedMessage);"),

// true
wrap("true(result);"),
bmish marked this conversation as resolved.
Show resolved Hide resolved
wrap("true(result, 'Message');"),
bmish marked this conversation as resolved.
Show resolved Hide resolved

// notOk
wrap("notOk(result);"),
wrap("notOk(result, 'Result is true');"),
Expand Down Expand Up @@ -279,6 +287,50 @@ ruleTester.run("assert-args", rule, {
}]
},

// false
{
code: wrap("assert.false();"),
errors: [{
messageId: "unexpectedArgCountNoMessage",
data: {
callee: "assert.false",
argCount: 0
}
}]
},
{
code: wrap("assert.false(a, b, 'Message');"),
errors: [{
messageId: "unexpectedArgCount",
data: {
callee: "assert.false",
argCount: 3
}
}]
},

// true
{
code: wrap("assert.true();"),
errors: [{
messageId: "unexpectedArgCountNoMessage",
data: {
callee: "assert.true",
argCount: 0
}
}]
},
{
code: wrap("assert.true(a, b, 'Message');"),
errors: [{
messageId: "unexpectedArgCount",
data: {
callee: "assert.true",
argCount: 3
}
}]
},

// strictEqual
{
code: wrap("strictEqual();"),
Expand Down
28 changes: 28 additions & 0 deletions tests/lib/rules/no-assert-logical-expression.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ ruleTester.run("no-assert-logical-expression", rule, {
// Simple assertions
wrap("assert.ok(foo);"),
wrap("assert.equal(foo, bar);"),
wrap("assert.false(foo);"),
wrap("assert.strictEqual(foo, bar);"),
wrap("assert.deepEqual(foo, bar);"),
wrap("assert.propEqual(foo, bar);"),
Expand All @@ -40,6 +41,7 @@ ruleTester.run("no-assert-logical-expression", rule, {
wrap("assert.notPropEqual(foo, bar);"),
wrap("assert.raises(function () {}, /Message/);"),
wrap("assert.throws(function () {}, /Message/);"),
wrap("assert.true(foo);"),

// Logical expressions inside raises/throw blocks are fine
wrap("assert.raises(function () { throw (foo || bar); });"),
Expand Down Expand Up @@ -421,6 +423,32 @@ ruleTester.run("no-assert-logical-expression", rule, {
line: 1,
column: 72
}]
},

// Boolean assertions
{
code: wrap("assert.true(foo && bar);"),
errors: [{
messageId: "noLogicalOperator",
data: {
operator: "&&"
},
type: "LogicalExpression",
line: 1,
column: 52
}]
},
{
code: wrap("assert.false(foo && bar);"),
errors: [{
messageId: "noLogicalOperator",
data: {
operator: "&&"
},
type: "LogicalExpression",
line: 1,
column: 53
}]
}
]
});
1 change: 1 addition & 0 deletions tests/lib/rules/no-conditional-assertions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ ruleTester.run("no-conditional-assertions", rule, {
invalid: [
"if (foo) assert.ok(true);",
"if (foo) { assert.ok(true); }",
"if (foo) { assert.true(true); }",
"if (foo) {} else if (bar) assert.ok(true);",
"if (foo) {} else assert.ok(true);",
"foo ? assert.ok(true) : false",
Expand Down
2 changes: 2 additions & 0 deletions tests/lib/rules/no-global-assertions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ ruleTester.run("no-global-assertions", rule, {
valid: [
wrap("assert.ok(true);"),
wrap("assert.equal(a, b);"),
wrap("assert.false(foo);"),
wrap("assert.strictEqual(a, b);"),
wrap("assert.deepEqual(a, b);"),
wrap("assert.propEqual(a, b);"),
Expand All @@ -49,6 +50,7 @@ ruleTester.run("no-global-assertions", rule, {
wrap("assert.notPropEqual(a, b);"),
wrap("assert.raises(function () {}, TypeError);"),
wrap("assert.throws(function () {}, TypeError);"),
wrap("assert.true(foo);"),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to add invalid test cases to this rule for the boolean assertions. Maybe true and falsecan't be added as globals because they are reserved keywords in JS?

wrap("assert.expect(1);"),

// Global overridden by local import/declaration.
Expand Down