From 902e55f44b95a3bbdf186dad47cdd149fa8e6e63 Mon Sep 17 00:00:00 2001 From: eric wang Date: Sat, 20 Jun 2020 16:00:59 +1000 Subject: [PATCH 1/3] New: Allow mocking the cwd in rule tester --- lib/rule-tester/rule-tester.js | 48 ++++++++++++++++++++-------- tests/lib/rule-tester/rule-tester.js | 24 +++++++++++++- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index 3e391576716..c97bacfd9ea 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -120,7 +120,8 @@ const RuleTesterParameters = [ "filename", "options", "errors", - "output" + "output", + "cwd" ]; /* @@ -337,7 +338,12 @@ class RuleTester { * @type {Object} */ this.rules = {}; - this.linter = new Linter(); + + /** + * Linter map, cwd -> Linter instance + * @type {Map} + */ + this.linterMap = new Map(); } /** @@ -425,7 +431,8 @@ class RuleTester { const testerConfig = this.testerConfig, requiredScenarios = ["valid", "invalid"], scenarioErrors = [], - linter = this.linter; + linterMap = this.linterMap, + rules = this.rules; if (lodash.isNil(test) || typeof test !== "object") { throw new TypeError(`Test Scenarios for rule ${ruleName} : Could not find test scenario object`); @@ -443,20 +450,34 @@ class RuleTester { ].concat(scenarioErrors).join("\n")); } + /** + * get linter from linterMap according to cwd + * @param {string} [cwd] cwd + * @returns {Linter} Linter instance + * @private + */ + function getLinter(cwd) { + if (!linterMap.has(cwd)) { + const linter = new Linter({ cwd }); - linter.defineRule(ruleName, Object.assign({}, rule, { + linterMap.set(cwd, linter); + } + const linter = linterMap.get(cwd); - // Create a wrapper rule that freezes the `context` properties. - create(context) { - freezeDeeply(context.options); - freezeDeeply(context.settings); - freezeDeeply(context.parserOptions); + linter.defineRule(ruleName, Object.assign({}, rule, { - return (typeof rule === "function" ? rule : rule.create)(context); - } - })); + // Create a wrapper rule that freezes the `context` properties. + create(context) { + freezeDeeply(context.options); + freezeDeeply(context.settings); + freezeDeeply(context.parserOptions); - linter.defineRules(this.rules); + return (typeof rule === "function" ? rule : rule.create)(context); + } + })); + linter.defineRules(rules); + return linter; + } /** * Run the rule for the given item @@ -467,6 +488,7 @@ class RuleTester { function runRuleForItem(item) { let config = lodash.cloneDeep(testerConfig), code, filename, output, beforeAST, afterAST; + const linter = getLinter(item.cwd); if (typeof item === "string") { code = item; diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 7e26e1a826c..6f3b880aa0c 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -751,7 +751,15 @@ describe("RuleTester", () => { }); it("should pass-through the parser to the rule", () => { - const spy = sinon.spy(ruleTester.linter, "verify"); + + // To generate the default linter + ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), { + valid: ["Eval(foo)"], + invalid: [] + }); + + const linter = ruleTester.linterMap.get(); + const spy = sinon.spy(linter, "verify"); ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), { valid: [ @@ -890,6 +898,20 @@ describe("RuleTester", () => { }, /Property "env" is the wrong type./u); }); + it("should pass-through the cwd to the linter", () => { + ruleTester.run("some-random-rule", ctx => { + + assert.strictEqual(ctx.getCwd(), "myCwd"); + return {}; + }, { + valid: [{ + code: "var test = 'foo'", + cwd: "myCwd" + }], + invalid: [] + }); + }); + it("should pass-through the tester config to the rule", () => { ruleTester = new RuleTester({ globals: { test: true } From 66d011f9bc12d712344c8984d2eb59271842231f Mon Sep 17 00:00:00 2001 From: eric wang Date: Sun, 21 Jun 2020 16:43:54 +1000 Subject: [PATCH 2/3] run set rules once --- lib/rule-tester/rule-tester.js | 46 ++++++++++++++-------------- tests/lib/rule-tester/rule-tester.js | 19 ++++++------ 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index c97bacfd9ea..a891d127001 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -338,12 +338,6 @@ class RuleTester { * @type {Object} */ this.rules = {}; - - /** - * Linter map, cwd -> Linter instance - * @type {Map} - */ - this.linterMap = new Map(); } /** @@ -422,8 +416,10 @@ class RuleTester { * @param {Function} rule The rule to test. * @param {{ * valid: (ValidTestCase | string)[], - * invalid: InvalidTestCase[] + * invalid: InvalidTestCase[], + * linterMapDevOnly?: Map * }} test The collection of tests to run. + * test.linterMapDevOnly is for testing purpose * @returns {void} */ run(ruleName, rule, test) { @@ -431,13 +427,19 @@ class RuleTester { const testerConfig = this.testerConfig, requiredScenarios = ["valid", "invalid"], scenarioErrors = [], - linterMap = this.linterMap, rules = this.rules; if (lodash.isNil(test) || typeof test !== "object") { throw new TypeError(`Test Scenarios for rule ${ruleName} : Could not find test scenario object`); } + /** + * Linter map, cwd -> Linter instance + * Injection of value in `test` is for test purpose. + * @type {Map} + */ + const { linterMapDevOnly: linterMap = new Map() } = test; + requiredScenarios.forEach(scenarioType => { if (lodash.isNil(test[scenarioType])) { scenarioErrors.push(`Could not find any ${scenarioType} test scenarios`); @@ -460,23 +462,21 @@ class RuleTester { if (!linterMap.has(cwd)) { const linter = new Linter({ cwd }); - linterMap.set(cwd, linter); - } - const linter = linterMap.get(cwd); + linter.defineRule(ruleName, Object.assign({}, rule, { - linter.defineRule(ruleName, Object.assign({}, rule, { + // Create a wrapper rule that freezes the `context` properties. + create(context) { + freezeDeeply(context.options); + freezeDeeply(context.settings); + freezeDeeply(context.parserOptions); - // Create a wrapper rule that freezes the `context` properties. - create(context) { - freezeDeeply(context.options); - freezeDeeply(context.settings); - freezeDeeply(context.parserOptions); - - return (typeof rule === "function" ? rule : rule.create)(context); - } - })); - linter.defineRules(rules); - return linter; + return (typeof rule === "function" ? rule : rule.create)(context); + } + })); + linter.defineRules(rules); + linterMap.set(cwd, linter); + } + return linterMap.get(cwd); } /** diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 6f3b880aa0c..929f903eb2f 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -10,6 +10,7 @@ const sinon = require("sinon"), EventEmitter = require("events"), { RuleTester } = require("../../../lib/rule-tester"), + { Linter } = require("../../../lib/linter"), assert = require("chai").assert, nodeAssert = require("assert"), { noop } = require("lodash"); @@ -751,16 +752,15 @@ describe("RuleTester", () => { }); it("should pass-through the parser to the rule", () => { - - // To generate the default linter - ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), { - valid: ["Eval(foo)"], - invalid: [] - }); - - const linter = ruleTester.linterMap.get(); + const linter = new Linter(); const spy = sinon.spy(linter, "verify"); + const linterMap = new Map([ + // eslint-disable-next-line no-undefined + [undefined, linter] + ]); + // ruleTester = new RuleTester({ linterMapDevOnly: linterMap }); + ruleTester = new RuleTester(); ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), { valid: [ { @@ -773,7 +773,8 @@ describe("RuleTester", () => { parser: require.resolve("esprima"), errors: [{ line: 1 }] } - ] + ], + linterMapDevOnly: linterMap }); assert.strictEqual(spy.args[1][1].parser, require.resolve("esprima")); }); From 78a13930fb7335e664c60c361af87bb1ed8e7450 Mon Sep 17 00:00:00 2001 From: eric wang Date: Sun, 21 Jun 2020 17:44:30 +1000 Subject: [PATCH 3/3] hide linterMapSymbol --- lib/rule-tester/index.js | 4 +--- lib/rule-tester/rule-tester.js | 25 +++++++++++++++++++++---- tests/lib/rule-tester/rule-tester.js | 7 ++++--- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/rule-tester/index.js b/lib/rule-tester/index.js index f52d14027c5..35191635185 100644 --- a/lib/rule-tester/index.js +++ b/lib/rule-tester/index.js @@ -1,5 +1,3 @@ "use strict"; -module.exports = { - RuleTester: require("./rule-tester") -}; +module.exports = require("./rule-tester"); diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index a891d127001..ad4b87087b0 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -100,6 +100,12 @@ const espreePath = require.resolve("espree"); * @property {number} [endColumn] The 1-based column number of the reported end location. */ +/** + * The private data for `RuleTester` instance. + * @typedef {Object} RuleTesterInternalSlots + * @property {symbol} injectedLinterMapSymbol symbol key for injected linterMap + */ + //------------------------------------------------------------------------------ // Private Members //------------------------------------------------------------------------------ @@ -312,6 +318,14 @@ function describeDefaultHandler(text, method) { return method.call(this); } +/** + * The map to store private data. + * @type {RuleTesterInternalSlots} + */ +const internalSlotsMap = { + injectedLinterMapSymbol: Symbol("linterMap") +}; + class RuleTester { /** @@ -417,9 +431,9 @@ class RuleTester { * @param {{ * valid: (ValidTestCase | string)[], * invalid: InvalidTestCase[], - * linterMapDevOnly?: Map + * Symbol("linterMap")?: Map * }} test The collection of tests to run. - * test.linterMapDevOnly is for testing purpose + * test.[Symbol.for("linterMap")] is for testing purpose * @returns {void} */ run(ruleName, rule, test) { @@ -438,7 +452,7 @@ class RuleTester { * Injection of value in `test` is for test purpose. * @type {Map} */ - const { linterMapDevOnly: linterMap = new Map() } = test; + const { [internalSlotsMap.injectedLinterMapSymbol]: linterMap = new Map() } = test; requiredScenarios.forEach(scenarioType => { if (lodash.isNil(test[scenarioType])) { @@ -912,4 +926,7 @@ class RuleTester { RuleTester[DESCRIBE] = RuleTester[IT] = null; -module.exports = RuleTester; +module.exports = { + RuleTester, + internalSlotsMap +}; diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 929f903eb2f..c755727c093 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -9,7 +9,7 @@ //------------------------------------------------------------------------------ const sinon = require("sinon"), EventEmitter = require("events"), - { RuleTester } = require("../../../lib/rule-tester"), + { RuleTester, internalSlotsMap } = require("../../../lib/rule-tester"), { Linter } = require("../../../lib/linter"), assert = require("chai").assert, nodeAssert = require("assert"), @@ -759,7 +759,8 @@ describe("RuleTester", () => { [undefined, linter] ]); - // ruleTester = new RuleTester({ linterMapDevOnly: linterMap }); + const { injectedLinterMapSymbol } = internalSlotsMap; + ruleTester = new RuleTester(); ruleTester.run("no-eval", require("../../fixtures/testers/rule-tester/no-eval"), { valid: [ @@ -774,7 +775,7 @@ describe("RuleTester", () => { errors: [{ line: 1 }] } ], - linterMapDevOnly: linterMap + [injectedLinterMapSymbol]: linterMap }); assert.strictEqual(spy.args[1][1].parser, require.resolve("esprima")); });