From b9aabe34311f6189b87c9d8a1aa40f3513fed773 Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Mon, 18 Feb 2019 19:25:06 -0500 Subject: [PATCH] Chore: run fuzzer along with unit tests (#11404) This commit turns on our existing fuzzer tool to run a small number of iterations when the user runs `npm test`. This is intended to prevent bugs like https://github.com/eslint/eslint/issues/11402 where a rule completely breaks when it encounters a particular syntax, but the author doesn't think to test for that kind of syntax. When there are no fuzzing errors, this adds about 5 seconds to the `npm test` time. (When there are fuzzing errors, it takes more time because the fuzzer does extra work to try to find the minimal config that reproduces the bug.) The fuzzer actually has two modes: "crash" (where it only tries to detect rule crashes), and "autofix" (where it additionally tries to detect cases where a rule's autofixer creates a syntax error). For now, this PR just enables "crash" mode, because I remember "autofix" mode had some false positives (although they might have been fixed due to parser upgrades). --- Makefile.js | 11 ++++++++--- tools/eslint-fuzzer.js | 13 +++++++------ tools/fuzzer-runner.js | 19 +++++++++---------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/Makefile.js b/Makefile.js index 99d0052680f..f49b06fd75d 100644 --- a/Makefile.js +++ b/Makefile.js @@ -540,12 +540,15 @@ target.lint = function() { } }; -target.fuzz = function() { +target.fuzz = function({ amount = process.env.CI ? 1000 : 300, fuzzBrokenAutofixes = true } = {}) { const fuzzerRunner = require("./tools/fuzzer-runner"); - const fuzzResults = fuzzerRunner.run({ amount: process.env.CI ? 1000 : 300 }); + const fuzzResults = fuzzerRunner.run({ amount, fuzzBrokenAutofixes }); if (fuzzResults.length) { - echo(`The fuzzer reported ${fuzzResults.length} error${fuzzResults.length === 1 ? "" : "s"}.`); + + const uniqueStackTraceCount = new Set(fuzzResults.map(result => result.error)).size; + + echo(`The fuzzer reported ${fuzzResults.length} error${fuzzResults.length === 1 ? "" : "s"} with a total of ${uniqueStackTraceCount} unique stack trace${uniqueStackTraceCount === 1 ? "" : "s"}.`); const formattedResults = JSON.stringify({ results: fuzzResults }, null, 4); @@ -606,6 +609,8 @@ target.test = function() { exit(1); } + target.fuzz({ amount: 150, fuzzBrokenAutofixes: false }); + target.checkLicenses(); }; diff --git a/tools/eslint-fuzzer.js b/tools/eslint-fuzzer.js index 342dee3e46f..153e654c7c5 100644 --- a/tools/eslint-fuzzer.js +++ b/tools/eslint-fuzzer.js @@ -29,7 +29,7 @@ const ruleConfigs = require("../lib/config/config-rule").createCoreRuleConfigs() * might also be passed other keys. * @param {boolean} [options.checkAutofixes=true] `true` if the fuzzer should check for autofix bugs. The fuzzer runs * roughly 4 times slower with autofix checking enabled. - * @param {function()} [options.progressCallback] A function that gets called once for each code sample + * @param {function(number)} [options.progressCallback] A function that gets called once for each code sample, with the total number of errors found so far * @returns {Object[]} A list of problems found. Each problem has the following properties: * type (string): The type of problem. This is either "crash" (a rule crashes) or "autofix" (an autofix produces a syntax error) * text (string): The text that ESLint should be run on to reproduce the problem @@ -52,10 +52,11 @@ function fuzz(options) { * Tries to isolate the smallest config that reproduces a problem * @param {string} text The source text to lint * @param {Object} config A config object that causes a crash or autofix error + * @param {("crash"|"autofix")} problemType The type of problem that occurred * @returns {Object} A config object with only one rule enabled that produces the same crash or autofix error, if possible. * Otherwise, the same as `config` */ - function isolateBadConfig(text, config) { + function isolateBadConfig(text, config, problemType) { for (const ruleId of Object.keys(config.rules)) { const reducedConfig = Object.assign({}, config, { rules: { [ruleId]: config.rules[ruleId] } }); let fixResult; @@ -66,7 +67,7 @@ function fuzz(options) { return reducedConfig; } - if (fixResult.messages.length === 1 && fixResult.messages[0].fatal) { + if (fixResult.messages.length === 1 && fixResult.messages[0].fatal && problemType === "autofix") { return reducedConfig; } } @@ -105,7 +106,7 @@ function fuzz(options) { const problems = []; - for (let i = 0; i < options.count; progressCallback(), i++) { + for (let i = 0; i < options.count; progressCallback(problems.length), i++) { const sourceType = lodash.sample(["script", "module"]); const text = codeGenerator({ sourceType }); const config = { @@ -122,14 +123,14 @@ function fuzz(options) { linter.verify(text, config); } } catch (err) { - problems.push({ type: "crash", text, config: isolateBadConfig(text, config), error: err.stack }); + problems.push({ type: "crash", text, config: isolateBadConfig(text, config, "crash"), error: err.stack }); continue; } if (checkAutofixes && autofixResult.fixed && autofixResult.messages.length === 1 && autofixResult.messages[0].fatal) { const lastGoodText = isolateBadAutofixPass(text, config); - problems.push({ type: "autofix", text: lastGoodText, config: isolateBadConfig(lastGoodText, config), error: autofixResult.messages[0] }); + problems.push({ type: "autofix", text: lastGoodText, config: isolateBadConfig(lastGoodText, config, "autofix"), error: autofixResult.messages[0] }); } } diff --git a/tools/fuzzer-runner.js b/tools/fuzzer-runner.js index 120f48c2880..b056c701f9e 100644 --- a/tools/fuzzer-runner.js +++ b/tools/fuzzer-runner.js @@ -34,33 +34,32 @@ const CRASH_AUTOFIX_TEST_COUNT_RATIO = 3; * @param {number} [options.amount=300] A positive integer indicating how much testing to do. Larger values result in a higher * chance of finding bugs, but cause the testing to take longer (linear increase). With the default value, the fuzzer * takes about 15 seconds to run. + * @param {boolean} [options.fuzzBrokenAutofixes=true] true if the fuzzer should look for invalid autofixes in addition to rule crashes * @returns {Object[]} A list of objects, where each object represents a problem detected by the fuzzer. The objects have the same * schema as objects returned from eslint-fuzzer. */ -function run(options) { - const amount = options && options.amount || 300; - +function run({ amount = 300, fuzzBrokenAutofixes = true } = {}) { const crashTestCount = amount * CRASH_AUTOFIX_TEST_COUNT_RATIO; - const autofixTestCount = amount; + const autofixTestCount = fuzzBrokenAutofixes ? amount : 0; /* * To keep the progress bar moving at a roughly constant speed, apply a different weight for finishing * a crash-only fuzzer run versus an autofix fuzzer run. */ const progressBar = new ProgressBar( - "Fuzzing rules [:bar] :percent, :elapseds elapsed, eta :etas", + "Fuzzing rules [:bar] :percent, :elapseds elapsed, eta :etas, errors so far: :elapsedErrors", { width: 30, total: crashTestCount + autofixTestCount * ESTIMATED_CRASH_AUTOFIX_PERFORMANCE_RATIO } ); // Start displaying the progress bar. - progressBar.tick(0); + progressBar.tick(0, { elapsedErrors: 0 }); const crashTestResults = fuzz({ linter, count: crashTestCount, checkAutofixes: false, - progressCallback: () => { - progressBar.tick(1); + progressCallback: elapsedErrors => { + progressBar.tick(1, { elapsedErrors }); progressBar.render(); } }); @@ -69,8 +68,8 @@ function run(options) { linter, count: autofixTestCount, checkAutofixes: true, - progressCallback: () => { - progressBar.tick(ESTIMATED_CRASH_AUTOFIX_PERFORMANCE_RATIO); + progressCallback: elapsedErrors => { + progressBar.tick(ESTIMATED_CRASH_AUTOFIX_PERFORMANCE_RATIO, { elapsedErrors: crashTestResults.length + elapsedErrors }); progressBar.render(); } });