Skip to content

Commit

Permalink
feat: Hooks for test cases (RuleTester) (#18771)
Browse files Browse the repository at this point in the history
* Add setup next to options.

* The prop type assertion.

* Execute if present.

* Positive test.

* Negative test.

* docs: Listing the property.

* Renaming `setup` to `before` according to RFC

* Testing corner case one: before() throws.

* Introducing the hook after(), draft impl.

* Add `after` to API docs.

* Ref: DNRY: running similar tests in Array::forEach().

* Ref: DNRY: extracting the hook runner into runHook() helper.

* Moving call of the after() hook, using try..finally, testing last corner cases.

* Moving call of before() into the try..finaly for code clarity and consistency.

* Testing that hooks are called both for valid and invalid cases.

* CR suggestion accepted: using strictEqual in runHook()

Co-authored-by: Nicholas C. Zakas <[email protected]>

* CR accepted in docs: rm article

Co-authored-by: Josh Goldberg ✨ <[email protected]>

* CR: typo fix in sample error message.

Co-authored-by: Milos Djermanovic <[email protected]>

* CR: Testing invalid cases in all tests.

* CR: Additional test for syntax errors.

---------

Co-authored-by: Nicholas C. Zakas <[email protected]>
Co-authored-by: Josh Goldberg ✨ <[email protected]>
Co-authored-by: Milos Djermanovic <[email protected]>
  • Loading branch information
4 people authored Sep 26, 2024
1 parent 493348a commit 17a07fb
Show file tree
Hide file tree
Showing 3 changed files with 192 additions and 2 deletions.
2 changes: 2 additions & 0 deletions docs/src/integrate/nodejs-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,8 @@ A test case is an object with the following properties:
* `name` (string, optional): The name to use for the test case, to make it easier to find
* `code` (string, required): The source code that the rule should be run on
* `options` (array, optional): The options passed to the rule. The rule severity should not be included in this list.
* `before` (function, optional): Function to execute before testing the case.
* `after` (function, optional): Function to execute after testing the case regardless of its result.
* `filename` (string, optional): The filename for the given case (useful for rules that make assertions about filenames).
* `only` (boolean, optional): Run this case exclusively for debugging in supported test frameworks.

Expand Down
35 changes: 33 additions & 2 deletions lib/rule-tester/rule-tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ const { SourceCode } = require("../languages/js/source-code");
* @property {string} [name] Name for the test case.
* @property {string} code Code for the test case.
* @property {any[]} [options] Options for the test case.
* @property {Function} [before] Function to execute before testing the case.
* @property {Function} [after] Function to execute after testing the case regardless of its result.
* @property {LanguageOptions} [languageOptions] The language options to use in the test case.
* @property {{ [name: string]: any }} [settings] Settings for the test case.
* @property {string} [filename] The fake filename for the test case. Useful for rules that make assertion about filenames.
Expand All @@ -61,6 +63,8 @@ const { SourceCode } = require("../languages/js/source-code");
* @property {number | Array<TestCaseError | string | RegExp>} errors Expected errors.
* @property {string | null} [output] The expected code after autofixes are applied. If set to `null`, the test runner will assert that no autofix is suggested.
* @property {any[]} [options] Options for the test case.
* @property {Function} [before] Function to execute before testing the case.
* @property {Function} [after] Function to execute after testing the case regardless of its result.
* @property {{ [name: string]: any }} [settings] Settings for the test case.
* @property {string} [filename] The fake filename for the test case. Useful for rules that make assertion about filenames.
* @property {LanguageOptions} [languageOptions] The language options to use in the test case.
Expand Down Expand Up @@ -105,6 +109,8 @@ const RuleTesterParameters = [
"code",
"filename",
"options",
"before",
"after",
"errors",
"output",
"only"
Expand Down Expand Up @@ -621,6 +627,21 @@ class RuleTester {
...defaultConfig.slice(1)
];

/**
* Runs a hook on the given item when it's assigned to the given property
* @param {string|Object} item Item to run the hook on
* @param {string} prop The property having the hook assigned to
* @throws {Error} If the property is not a function or that function throws an error
* @returns {void}
* @private
*/
function runHook(item, prop) {
if (typeof item === "object" && hasOwnProperty(item, prop)) {
assert.strictEqual(typeof item[prop], "function", `Optional test case property '${prop}' must be a function`);
item[prop]();
}
}

/**
* Run the rule for the given item
* @param {string|Object} item Item to run the rule against
Expand Down Expand Up @@ -1258,7 +1279,12 @@ class RuleTester {
this.constructor[valid.only ? "itOnly" : "it"](
sanitize(typeof valid === "object" ? valid.name || valid.code : valid),
() => {
testValidTemplate(valid);
try {
runHook(valid, "before");
testValidTemplate(valid);
} finally {
runHook(valid, "after");
}
}
);
});
Expand All @@ -1271,7 +1297,12 @@ class RuleTester {
this.constructor[invalid.only ? "itOnly" : "it"](
sanitize(invalid.name || invalid.code),
() => {
testInvalidTemplate(invalid);
try {
runHook(invalid, "before");
testInvalidTemplate(invalid);
} finally {
runHook(invalid, "after");
}
}
);
});
Expand Down
157 changes: 157 additions & 0 deletions tests/lib/rule-tester/rule-tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,163 @@ describe("RuleTester", () => {
});
});

describe("hooks", () => {
const ruleName = "no-var";
const rule = require("../../fixtures/testers/rule-tester/no-var");

["before", "after"].forEach(hookName => {
it(`${hookName} should be called when a function is assigned`, () => {
const hookForValid = sinon.stub();
const hookForInvalid = sinon.stub();

ruleTester = new RuleTester();
ruleTester.run(ruleName, rule, {
valid: [{
code: "const onlyValid = 42;",
[hookName]: hookForValid
}],
invalid: [{
code: "var onlyValid = 42;",
errors: [/^Bad var/u],
output: " onlyValid = 42;",
[hookName]: hookForInvalid
}]
});
sinon.assert.calledOnce(hookForValid);
sinon.assert.calledOnce(hookForInvalid);
});

it(`${hookName} should cause test to fail when it throws error`, () => {
const hook = sinon.stub().throws(new Error("Something happened"));

ruleTester = new RuleTester();
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [{
code: "const onlyValid = 42;",
[hookName]: hook
}],
invalid: []
}), "Something happened");
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [],
invalid: [{
code: "var onlyValid = 42;",
errors: [/^Bad var/u],
output: " onlyValid = 42;",
[hookName]: hook
}]
}), "Something happened");
});

it(`${hookName} should throw when not a function is assigned`, () => {
ruleTester = new RuleTester();
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [{
code: "const onlyValid = 42;",
[hookName]: 42
}],
invalid: []
}), `Optional test case property '${hookName}' must be a function`);
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [],
invalid: [{
code: "var onlyValid = 42;",
errors: [/^Bad var/u],
output: " onlyValid = 42;",
[hookName]: 42
}]
}), `Optional test case property '${hookName}' must be a function`);
});
});

it("should call both before() and after() hooks even when the case failed", () => {
const hookBefore = sinon.stub();
const hookAfter = sinon.stub();

ruleTester = new RuleTester();
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [{
code: "var onlyValid = 42;",
before: hookBefore,
after: hookAfter
}],
invalid: []
}));
sinon.assert.calledOnce(hookBefore);
sinon.assert.calledOnce(hookAfter);
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [],
invalid: [{
code: "const onlyValid = 42;",
errors: [/^Bad var/u],
output: " onlyValid = 42;",
before: hookBefore,
after: hookAfter
}]
}));
sinon.assert.calledTwice(hookBefore);
sinon.assert.calledTwice(hookAfter);
});

it("should call both before() and after() hooks regardless syntax errors", () => {
const hookBefore = sinon.stub();
const hookAfter = sinon.stub();

ruleTester = new RuleTester();
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [{
code: "invalid javascript code",
before: hookBefore,
after: hookAfter
}],
invalid: []
}), /parsing error/u);
sinon.assert.calledOnce(hookBefore);
sinon.assert.calledOnce(hookAfter);
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [],
invalid: [{
code: "invalid javascript code",
errors: [/^Bad var/u],
output: " onlyValid = 42;",
before: hookBefore,
after: hookAfter
}]
}), /parsing error/u);
sinon.assert.calledTwice(hookBefore);
sinon.assert.calledTwice(hookAfter);
});

it("should call after() hook even when before() throws", () => {
const hookBefore = sinon.stub().throws(new Error("Something happened in before()"));
const hookAfter = sinon.stub();

ruleTester = new RuleTester();
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [{
code: "const onlyValid = 42;",
before: hookBefore,
after: hookAfter
}],
invalid: []
}), "Something happened in before()");
sinon.assert.calledOnce(hookBefore);
sinon.assert.calledOnce(hookAfter);
assert.throws(() => ruleTester.run(ruleName, rule, {
valid: [],
invalid: [{
code: "var onlyValid = 42;",
errors: [/^Bad var/u],
output: " onlyValid = 42;",
before: hookBefore,
after: hookAfter
}]
}), "Something happened in before()");
sinon.assert.calledTwice(hookBefore);
sinon.assert.calledTwice(hookAfter);
});
});

it("should not throw an error when everything passes", () => {
ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), {
valid: [
Expand Down

0 comments on commit 17a07fb

Please sign in to comment.