Skip to content

Commit 17a07fb

Browse files
RobinTailnzakasJoshuaKGoldbergmdjermanovic
authored
feat: Hooks for test cases (RuleTester) (#18771)
* 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 <nicholas@humanwhocodes.com> * CR accepted in docs: rm article Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com> * CR: typo fix in sample error message. Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * CR: Testing invalid cases in all tests. * CR: Additional test for syntax errors. --------- Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com> Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com> Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
1 parent 493348a commit 17a07fb

3 files changed

Lines changed: 192 additions & 2 deletions

File tree

‎docs/src/integrate/nodejs-api.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,8 @@ A test case is an object with the following properties:
818818
* `name` (string, optional): The name to use for the test case, to make it easier to find
819819
* `code` (string, required): The source code that the rule should be run on
820820
* `options` (array, optional): The options passed to the rule. The rule severity should not be included in this list.
821+
* `before` (function, optional): Function to execute before testing the case.
822+
* `after` (function, optional): Function to execute after testing the case regardless of its result.
821823
* `filename` (string, optional): The filename for the given case (useful for rules that make assertions about filenames).
822824
* `only` (boolean, optional): Run this case exclusively for debugging in supported test frameworks.
823825

‎lib/rule-tester/rule-tester.js‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ const { SourceCode } = require("../languages/js/source-code");
4747
* @property {string} [name] Name for the test case.
4848
* @property {string} code Code for the test case.
4949
* @property {any[]} [options] Options for the test case.
50+
* @property {Function} [before] Function to execute before testing the case.
51+
* @property {Function} [after] Function to execute after testing the case regardless of its result.
5052
* @property {LanguageOptions} [languageOptions] The language options to use in the test case.
5153
* @property {{ [name: string]: any }} [settings] Settings for the test case.
5254
* @property {string} [filename] The fake filename for the test case. Useful for rules that make assertion about filenames.
@@ -61,6 +63,8 @@ const { SourceCode } = require("../languages/js/source-code");
6163
* @property {number | Array<TestCaseError | string | RegExp>} errors Expected errors.
6264
* @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.
6365
* @property {any[]} [options] Options for the test case.
66+
* @property {Function} [before] Function to execute before testing the case.
67+
* @property {Function} [after] Function to execute after testing the case regardless of its result.
6468
* @property {{ [name: string]: any }} [settings] Settings for the test case.
6569
* @property {string} [filename] The fake filename for the test case. Useful for rules that make assertion about filenames.
6670
* @property {LanguageOptions} [languageOptions] The language options to use in the test case.
@@ -105,6 +109,8 @@ const RuleTesterParameters = [
105109
"code",
106110
"filename",
107111
"options",
112+
"before",
113+
"after",
108114
"errors",
109115
"output",
110116
"only"
@@ -621,6 +627,21 @@ class RuleTester {
621627
...defaultConfig.slice(1)
622628
];
623629

630+
/**
631+
* Runs a hook on the given item when it's assigned to the given property
632+
* @param {string|Object} item Item to run the hook on
633+
* @param {string} prop The property having the hook assigned to
634+
* @throws {Error} If the property is not a function or that function throws an error
635+
* @returns {void}
636+
* @private
637+
*/
638+
function runHook(item, prop) {
639+
if (typeof item === "object" && hasOwnProperty(item, prop)) {
640+
assert.strictEqual(typeof item[prop], "function", `Optional test case property '${prop}' must be a function`);
641+
item[prop]();
642+
}
643+
}
644+
624645
/**
625646
* Run the rule for the given item
626647
* @param {string|Object} item Item to run the rule against
@@ -1258,7 +1279,12 @@ class RuleTester {
12581279
this.constructor[valid.only ? "itOnly" : "it"](
12591280
sanitize(typeof valid === "object" ? valid.name || valid.code : valid),
12601281
() => {
1261-
testValidTemplate(valid);
1282+
try {
1283+
runHook(valid, "before");
1284+
testValidTemplate(valid);
1285+
} finally {
1286+
runHook(valid, "after");
1287+
}
12621288
}
12631289
);
12641290
});
@@ -1271,7 +1297,12 @@ class RuleTester {
12711297
this.constructor[invalid.only ? "itOnly" : "it"](
12721298
sanitize(invalid.name || invalid.code),
12731299
() => {
1274-
testInvalidTemplate(invalid);
1300+
try {
1301+
runHook(invalid, "before");
1302+
testInvalidTemplate(invalid);
1303+
} finally {
1304+
runHook(invalid, "after");
1305+
}
12751306
}
12761307
);
12771308
});

‎tests/lib/rule-tester/rule-tester.js‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,163 @@ describe("RuleTester", () => {
463463
});
464464
});
465465

466+
describe("hooks", () => {
467+
const ruleName = "no-var";
468+
const rule = require("../../fixtures/testers/rule-tester/no-var");
469+
470+
["before", "after"].forEach(hookName => {
471+
it(`${hookName} should be called when a function is assigned`, () => {
472+
const hookForValid = sinon.stub();
473+
const hookForInvalid = sinon.stub();
474+
475+
ruleTester = new RuleTester();
476+
ruleTester.run(ruleName, rule, {
477+
valid: [{
478+
code: "const onlyValid = 42;",
479+
[hookName]: hookForValid
480+
}],
481+
invalid: [{
482+
code: "var onlyValid = 42;",
483+
errors: [/^Bad var/u],
484+
output: " onlyValid = 42;",
485+
[hookName]: hookForInvalid
486+
}]
487+
});
488+
sinon.assert.calledOnce(hookForValid);
489+
sinon.assert.calledOnce(hookForInvalid);
490+
});
491+
492+
it(`${hookName} should cause test to fail when it throws error`, () => {
493+
const hook = sinon.stub().throws(new Error("Something happened"));
494+
495+
ruleTester = new RuleTester();
496+
assert.throws(() => ruleTester.run(ruleName, rule, {
497+
valid: [{
498+
code: "const onlyValid = 42;",
499+
[hookName]: hook
500+
}],
501+
invalid: []
502+
}), "Something happened");
503+
assert.throws(() => ruleTester.run(ruleName, rule, {
504+
valid: [],
505+
invalid: [{
506+
code: "var onlyValid = 42;",
507+
errors: [/^Bad var/u],
508+
output: " onlyValid = 42;",
509+
[hookName]: hook
510+
}]
511+
}), "Something happened");
512+
});
513+
514+
it(`${hookName} should throw when not a function is assigned`, () => {
515+
ruleTester = new RuleTester();
516+
assert.throws(() => ruleTester.run(ruleName, rule, {
517+
valid: [{
518+
code: "const onlyValid = 42;",
519+
[hookName]: 42
520+
}],
521+
invalid: []
522+
}), `Optional test case property '${hookName}' must be a function`);
523+
assert.throws(() => ruleTester.run(ruleName, rule, {
524+
valid: [],
525+
invalid: [{
526+
code: "var onlyValid = 42;",
527+
errors: [/^Bad var/u],
528+
output: " onlyValid = 42;",
529+
[hookName]: 42
530+
}]
531+
}), `Optional test case property '${hookName}' must be a function`);
532+
});
533+
});
534+
535+
it("should call both before() and after() hooks even when the case failed", () => {
536+
const hookBefore = sinon.stub();
537+
const hookAfter = sinon.stub();
538+
539+
ruleTester = new RuleTester();
540+
assert.throws(() => ruleTester.run(ruleName, rule, {
541+
valid: [{
542+
code: "var onlyValid = 42;",
543+
before: hookBefore,
544+
after: hookAfter
545+
}],
546+
invalid: []
547+
}));
548+
sinon.assert.calledOnce(hookBefore);
549+
sinon.assert.calledOnce(hookAfter);
550+
assert.throws(() => ruleTester.run(ruleName, rule, {
551+
valid: [],
552+
invalid: [{
553+
code: "const onlyValid = 42;",
554+
errors: [/^Bad var/u],
555+
output: " onlyValid = 42;",
556+
before: hookBefore,
557+
after: hookAfter
558+
}]
559+
}));
560+
sinon.assert.calledTwice(hookBefore);
561+
sinon.assert.calledTwice(hookAfter);
562+
});
563+
564+
it("should call both before() and after() hooks regardless syntax errors", () => {
565+
const hookBefore = sinon.stub();
566+
const hookAfter = sinon.stub();
567+
568+
ruleTester = new RuleTester();
569+
assert.throws(() => ruleTester.run(ruleName, rule, {
570+
valid: [{
571+
code: "invalid javascript code",
572+
before: hookBefore,
573+
after: hookAfter
574+
}],
575+
invalid: []
576+
}), /parsing error/u);
577+
sinon.assert.calledOnce(hookBefore);
578+
sinon.assert.calledOnce(hookAfter);
579+
assert.throws(() => ruleTester.run(ruleName, rule, {
580+
valid: [],
581+
invalid: [{
582+
code: "invalid javascript code",
583+
errors: [/^Bad var/u],
584+
output: " onlyValid = 42;",
585+
before: hookBefore,
586+
after: hookAfter
587+
}]
588+
}), /parsing error/u);
589+
sinon.assert.calledTwice(hookBefore);
590+
sinon.assert.calledTwice(hookAfter);
591+
});
592+
593+
it("should call after() hook even when before() throws", () => {
594+
const hookBefore = sinon.stub().throws(new Error("Something happened in before()"));
595+
const hookAfter = sinon.stub();
596+
597+
ruleTester = new RuleTester();
598+
assert.throws(() => ruleTester.run(ruleName, rule, {
599+
valid: [{
600+
code: "const onlyValid = 42;",
601+
before: hookBefore,
602+
after: hookAfter
603+
}],
604+
invalid: []
605+
}), "Something happened in before()");
606+
sinon.assert.calledOnce(hookBefore);
607+
sinon.assert.calledOnce(hookAfter);
608+
assert.throws(() => ruleTester.run(ruleName, rule, {
609+
valid: [],
610+
invalid: [{
611+
code: "var onlyValid = 42;",
612+
errors: [/^Bad var/u],
613+
output: " onlyValid = 42;",
614+
before: hookBefore,
615+
after: hookAfter
616+
}]
617+
}), "Something happened in before()");
618+
sinon.assert.calledTwice(hookBefore);
619+
sinon.assert.calledTwice(hookAfter);
620+
});
621+
});
622+
466623
it("should not throw an error when everything passes", () => {
467624
ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), {
468625
valid: [

0 commit comments

Comments
 (0)