From 0d14005322dd1df60da50550ec78a44f68f6adf4 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Sun, 16 Jan 2022 06:00:44 -0800 Subject: [PATCH 01/34] Autogenerate tests to make sure startLine and startColumn work everywhere. Also improve the performance of tests so that the fact that we have ~88,000 tests now doesn't really cause the tests to run any slower. Reviewed by @tolmasky. --- .github/workflows/ci.yml | 2 +- .../babel-parser/test/attachComment-false.js | 5 +- packages/babel-parser/test/estree-throws.js | 5 +- packages/babel-parser/test/expressions.js | 2 +- .../babel-parser/test/helpers/difference.js | 137 ++++++ .../test/helpers/fixture-error.js | 64 +++ .../babel-parser/test/helpers/polyfill.js | 32 ++ .../test/helpers/runFixtureTests.js | 395 ++++++------------ .../test/helpers/to-fuzzed-options.js | 99 +++++ packages/babel-parser/test/index.js | 2 +- 10 files changed, 462 insertions(+), 281 deletions(-) create mode 100644 packages/babel-parser/test/helpers/difference.js create mode 100644 packages/babel-parser/test/helpers/fixture-error.js create mode 100644 packages/babel-parser/test/helpers/polyfill.js create mode 100644 packages/babel-parser/test/helpers/to-fuzzed-options.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 58639b2fc309..a86304513628 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -188,7 +188,7 @@ jobs: # Todo(Babel 8): Jest execution path is hardcoded because Yarn 2 does not support node 6 run: | - BABEL_ENV=test node ./node_modules/.bin/jest --ci --color + BABEL_ENV=test node --max-old-space-size=4096 ./node_modules/.bin/jest --ci --color test-babel-8-breaking: name: Test Babel 8 breaking changes diff --git a/packages/babel-parser/test/attachComment-false.js b/packages/babel-parser/test/attachComment-false.js index 83c49b52c92c..ea61f229030e 100644 --- a/packages/babel-parser/test/attachComment-false.js +++ b/packages/babel-parser/test/attachComment-false.js @@ -1,12 +1,13 @@ import path from "path"; -import { runFixtureTestsWithoutExactASTMatch } from "./helpers/runFixtureTests.js"; +import runFixtureTests from "./helpers/runFixtureTests.js"; import { parseExpression } from "../lib/index.js"; import { fileURLToPath } from "url"; -runFixtureTestsWithoutExactASTMatch( +runFixtureTests( path.join(path.dirname(fileURLToPath(import.meta.url)), "expressions"), (input, options = {}) => { options.attachComment = false; return parseExpression(input, options); }, + true, ); diff --git a/packages/babel-parser/test/estree-throws.js b/packages/babel-parser/test/estree-throws.js index f8a4a7d14507..9e70e9cf365f 100644 --- a/packages/babel-parser/test/estree-throws.js +++ b/packages/babel-parser/test/estree-throws.js @@ -1,12 +1,13 @@ import path from "path"; -import { runFixtureTestsWithoutExactASTMatch } from "./helpers/runFixtureTests.js"; +import runFixtureTests from "./helpers/runFixtureTests.js"; import { parse } from "../lib/index.js"; import { fileURLToPath } from "url"; -runFixtureTestsWithoutExactASTMatch( +runFixtureTests( path.join(path.dirname(fileURLToPath(import.meta.url)), "fixtures"), (input, options = {}) => { const plugins = options.plugins || []; return parse(input, { ...options, plugins: plugins.concat("estree") }); }, + true, ); diff --git a/packages/babel-parser/test/expressions.js b/packages/babel-parser/test/expressions.js index 802deee1873f..c1dfd6d32407 100644 --- a/packages/babel-parser/test/expressions.js +++ b/packages/babel-parser/test/expressions.js @@ -1,5 +1,5 @@ import path from "path"; -import { runFixtureTests } from "./helpers/runFixtureTests.js"; +import runFixtureTests from "./helpers/runFixtureTests.js"; import { parseExpression } from "../lib/index.js"; import { fileURLToPath } from "url"; diff --git a/packages/babel-parser/test/helpers/difference.js b/packages/babel-parser/test/helpers/difference.js new file mode 100644 index 000000000000..daa54411e09e --- /dev/null +++ b/packages/babel-parser/test/helpers/difference.js @@ -0,0 +1,137 @@ +/* eslint-disable no-confusing-arrow */ + +import { isIdentifierName } from "@babel/helper-validator-identifier"; + +const { isArray } = Array; +const { hasOwnProperty } = Object; + +export default class Difference { + constructor(adjust, expected, actual) { + const woundDifference = compare(adjust, expected, actual); + + if (!woundDifference) { + return Difference.None; + } + + const [path, reason] = toUnwoundDifference(woundDifference); + const message = `${toExplanationString(reason)} in ${toPathString(path)}`; + + return Object.assign(this, { ...reason, path, message }); + } +} + +Difference.None = Object.freeze( + Object.setPrototypeOf({}, Difference.prototype), +); + +const toType = value => + value === null + ? "null" + : typeof value !== "object" + ? typeof value + : isArray(value) + ? "Array" + : value instanceof RegExp + ? "RegExp" + : "Object"; + +function compare(adjust, expected, actual) { + // easy. + if (Object.is(expected, actual)) { + return false; + } + + const typeExpected = toType(expected); + const typeActual = toType(actual); + + if (typeExpected !== typeActual) { + return { discrepancy: "value", expected, actual }; + } + + // Just ignore functions (AKA, assume they're equal). + if (typeActual === "function") { + return false; + } + + if (typeActual === "RegExp" && expected + "" === actual + "") { + return false; + } + + if (typeActual !== "Object" && typeActual !== "Array") { + return { discrepancy: "value", expected, actual }; + } + + const keysExpected = Object.keys(expected); + const keysActual = Object.keys(actual).filter( + key => actual[key] !== void 0 && typeof actual[key] !== "function", + ); + const lengthExpected = keysExpected.length; + const lengthActual = keysActual.length; + + if (lengthExpected !== lengthActual && typeActual === "Array") { + return { + discrepancy: "length", + expected: lengthExpected, + actual: lengthActual, + }; + } + + if (lengthExpected < lengthActual) { + const keysExpectedSet = new Set(keysExpected); + const key = keysActual.find(key => !keysExpectedSet.has(key)); + + if (key !== void 0) { + return { discrepancy: "unexpected-key", key }; + } + } + + for (const key of keysExpected) { + if (!hasOwnProperty.call(actual, key)) { + return { discrepancy: "missing-key", key, actual }; + } + + const original = expected[key]; + const adjusted = adjust ? adjust(original, key, expected) : original; + const difference = compare(adjust, adjusted, actual[key]); + if (difference) { + return [key, difference]; + } + } + + return false; +} + +const toUnwoundDifference = compiled => + !isArray(compiled) + ? [[], compiled] + : toUnwoundDifference(compiled[1]).map((item, index) => + index === 0 ? [compiled[0], ...item] : item, + ); + +const toValueString = (value, type = toType(value)) => + type === "string" + ? JSON.stringify(value) + : type === "symbol" + ? value.toString() + : type === "bigint" + ? `${value}n` + : type !== "number" || 1 / value !== -Infinity + ? value + "" + : "-0"; + +const toExplanationString = ({ discrepancy, expected, actual, key }) => + discrepancy === "length" + ? `Array of wrong size, expected length of ${expected}, but got ${actual}` + : discrepancy === "unexpected-key" + ? `Did not expect a property ${toValueString(key)}` + : discrepancy === "missing-key" + ? `${toType(actual)} is missing property ${toValueString(key)}` + : discrepancy === "type" + ? `Wrong type "${expected}" != "${actual}"` + : `${toValueString(expected)} != ${toValueString(actual)}`; + +const isInt = key => parseInt(key, 10) + "" === key; +const toAccess = key => + isInt(key) ? `[${key}]` : isIdentifierName(key) ? `.${key}` : `["${key}"]`; + +const toPathString = path => path.map(toAccess).join(""); diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js new file mode 100644 index 000000000000..53cb962444b8 --- /dev/null +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -0,0 +1,64 @@ +import "./polyfill.js"; + +const { defineProperty, entries, fromEntries } = Object; + +const named = (name, object) => defineProperty(object, "name", { value: name }); +const mapEntries = (object, f) => fromEntries(entries(object).map(f)); + +export default class FixtureError extends Error { + constructor(message, difference) { + super(message); + this.difference = difference; + } + + static fromDifference(difference, actual) { + return difference.path[0] !== "threw" + ? new FixtureError.DifferentAST(difference) + : !difference.expected + ? new FixtureError.UnexpectedError(difference) + : difference.actual + ? new FixtureError.DifferentError(difference) + : actual.ast.errors + ? new FixtureError.UnexpectedRecovery(difference, actual.ast.errors) + : new FixtureError.UnexpectedSuccess(difference); + } +} + +Object.assign( + FixtureError, + mapEntries( + { + DifferentError: ({ expected, actual }) => + `Expected error message:\n\n${expected}\n\n` + + `But instead found error message:\n\n${actual}`, + + DifferentAST: ({ message }) => message, + + UnexpectedError: ({ actual }) => + `Encountered unexpected non-recoverable message:\n\n${actual}`, + + UnexpectedSuccess: ({ expected }) => + `Expected non-recoverable error message:\n\n${expected}\n\n` + + `But parsing succeeded without errors.`, + + UnexpectedRecovery: ({ expected }, errors) => + `Expected non-recoverable error message:\n\n${expected}\n\n` + + `But instead parsing recovered from errors:\n\n${JSON.stringify( + errors, + null, + 2, + )}`, + }, + ([name, toMessage]) => [ + name, + named( + name, + class extends FixtureError { + constructor(difference, ...args) { + super(toMessage(difference, ...args), difference); + } + }, + ), + ], + ), +); diff --git a/packages/babel-parser/test/helpers/polyfill.js b/packages/babel-parser/test/helpers/polyfill.js new file mode 100644 index 000000000000..336fbeb22a4e --- /dev/null +++ b/packages/babel-parser/test/helpers/polyfill.js @@ -0,0 +1,32 @@ +// We run these tests as far back as Node 6, so we need these there. + +if (!Object.entries) { + Object.entries = object => Object.keys(object).map(key => [key, object[key]]); +} + +// From: https://github.com/tc39/proposal-object-from-entries/blob/main/polyfill.js +if (!Object.fromEntries) { + Object.fromEntries = function (entries) { + const obj = {}; + + for (const pair of entries) { + if (Object(pair) !== pair) { + throw new TypeError("iterable for fromEntries should yield objects"); + } + + // Consistency with Map: contract is that entry has "0" and "1" keys, not + // that it is an array or iterable. + + const { 0: key, 1: val } = pair; + + Object.defineProperty(obj, key, { + configurable: true, + enumerable: true, + writable: true, + value: val, + }); + } + + return obj; + }; +} diff --git a/packages/babel-parser/test/helpers/runFixtureTests.js b/packages/babel-parser/test/helpers/runFixtureTests.js index 7388a01d836c..9fbdc20b0497 100644 --- a/packages/babel-parser/test/helpers/runFixtureTests.js +++ b/packages/babel-parser/test/helpers/runFixtureTests.js @@ -2,13 +2,39 @@ import { multiple as getFixtures } from "@babel/helper-fixtures"; import { codeFrameColumns } from "@babel/code-frame"; import fs from "fs"; import path from "path"; -import { fileURLToPath } from "url"; - +// import { fileURLToPath } from "url"; +import Difference from "./difference.js"; +import FixtureError2 from "./fixture-error.js"; +import toFuzzedOptions from "./to-fuzzed-options.js"; + +const { parse: JSONParse, stringify } = JSON; + +// We've only serialized one BigInt in the entire test suite: +// +// packages/babel-parser/test/fixtures/estree/basic/bigint/output.json +// +// This is because only estree actually includes the BigInt value in the Literal +// node. If the JS environemnt doesn['t support bigint, then estree will just +// use null for the value. We also happen to just throw the AST information away +// with estree tests, so in the event that we're running on an older version of +// Node that doesn't support bigint, it is safe to deserialize to null. +const toBigInt = global.BigInt || (() => null); + +/* eslint-disable no-confusing-arrow */ +const deserialize = string => + JSONParse(string, (key, value) => + key !== "value" || !value || typeof value !== "object" || !value[serialized] + ? value + : value[serialized] === "RegExp" + ? new RegExp(value.source, value.flags) + : toBigInt(value.value), + ); +/* const rootPath = path.join( path.dirname(fileURLToPath(import.meta.url)), "../../../..", ); - +*/ const serialized = "$$ babel internal serialized type"; class FixtureError extends Error { @@ -45,295 +71,116 @@ class FixtureError extends Error { } } -export function runFixtureTests(fixturesPath, parseFunction) { - const fixtures = getFixtures(fixturesPath); - - Object.keys(fixtures).forEach(function (name) { - fixtures[name].forEach(function (testSuite) { - testSuite.tests.forEach(function (task) { - const testFn = task.disabled ? it.skip : it; - - testFn(name + "/" + testSuite.title + "/" + task.title, function () { - try { - runTest(task, parseFunction); - } catch (err) { - if (!task.expect.code && !process.env.CI) { - const fn = path.dirname(task.expect.loc) + "/options.json"; - if (!fs.existsSync(fn)) { - task.options = task.options || {}; - task.options.throws = err.message.replace( - /^.*Got error message: /, - "", - ); - fs.writeFileSync(fn, JSON.stringify(task.options, null, 2)); - } - } - - const fixturePath = `${path.relative( - rootPath, - fixturesPath, - )}/${name}/${task.actual.filename}`; - throw new FixtureError(err, fixturePath, task.actual.code); - } - }); - }); - }); - }); -} - -/** - * Run Fixture test without an exact AST match. If the output.json does not contain - * "errors", it asserts the actual output does not have "errors". If the output.json - * have "errors", it asserts the actual output have the same "errors". - * - * This routine is used to test parser options that have impact on the AST shape but - * does not change the syntax - * @param {*} fixturesPath The path to the fixture root - * @param {*} parseFunction The customized parseFunction, different global test options - * should be implemented here - */ -export function runFixtureTestsWithoutExactASTMatch( +export default function runFixtureTests( fixturesPath, parseFunction, + onlyCompareErrors = false, ) { const fixtures = getFixtures(fixturesPath); - Object.keys(fixtures).forEach(function (name) { - fixtures[name].forEach(function (testSuite) { - testSuite.tests.forEach(function (task) { - const testFn = task.disabled ? it.skip : it; - - testFn(name + "/" + testSuite.title + "/" + task.title, function () { - try { - runTest(task, parseFunction, true); - } catch (err) { - const fixturePath = `${path.relative( - rootPath, - fixturesPath, - )}/${name}/${task.actual.filename}`; - throw new FixtureError(err, fixturePath, task.actual.code); - } - }); - }); - }); - }); -} - -// compact loc properties into a single line -function compactFixture(jsonString) { - return jsonString.replace( - /"start": (\d+),\s+"end": (\d+),\s+"loc": \{\s+"start":\s\{\s+"line": (\d+),\s+"column": (\d+)\s+\},\s+"end":\s\{\s+"line": (\d+),\s+"column": (\d+)\s+\s+\}(?:,\s+"identifierName": "(\S+)")?\s+\}/gm, - (_, p1, p2, p3, p4, p5, p6, p7) => { - return ( - `"start":${p1},"end":${p2},"loc":{"start":{"line":${p3},"column":${p4}},"end":{"line":${p5},"column":${p6}}` + - (p7 ? `,"identifierName":"${p7}"}` : "}") - ); - }, - ); -} - -function save(test, ast) { - fs.writeFileSync( - test.expect.loc, - compactFixture(JSON.stringify(ast, (k, v) => serialize(v), 2)), - ); -} - -/** - * run parser on given tests - * - * @param {Test} A {@link packages/babel-helper-fixtures/src/index.js Test} instance - generated from `getFixtures` - * @param {*} parseFunction A parser with the same interface of `@babel/parser#parse` - * @param {boolean} [compareErrorsOnly=false] Whether we should only compare the "errors" - * of generated ast against the expected AST. Used for `runFixtureTestsWithoutExactASTMatch` where an - * ESTree AST is generated but we want to make sure `@babel/parser` still throws expected - * recoverable errors on given code locations. - * @returns {void} - */ -function runTest(test, parseFunction, compareErrorsOnly = false) { - const opts = test.options; - - if (opts.throws && test.expect.code) { - throw new Error( - "File expected.json exists although options specify throws. Remove expected.json.", - ); - } - - let ast; - try { - ast = parseFunction(test.actual.code, { errorRecovery: true, ...opts }); - } catch (err) { - if (opts.throws) { - if (err.message === opts.throws) { - return; - } else { - if (process.env.OVERWRITE) { - const fn = path.dirname(test.expect.loc) + "/options.json"; - test.options = test.options || {}; - test.options.throws = err.message; - fs.writeFileSync(fn, JSON.stringify(test.options, null, 2)); - return; - } - - err.message = - "Expected error message: " + - opts.throws + - ". Got error message: " + - err.message; - throw err; + for (const [name, testSuites] of Object.entries(fixtures)) { + for (const { title, tests } of testSuites) { + for (const test of tests) { + runAutogeneratedParseTests( + parseFunction, + `${name}/${title}`, + test, + onlyCompareErrors, + ); } } - - throw err; - } - - if (ast.comments && !ast.comments.length) delete ast.comments; - if (ast.errors && !ast.errors.length) delete ast.errors; - - if ( - !test.expect.code && - !opts.throws && - !process.env.CI && - !compareErrorsOnly - ) { - test.expect.loc += "on"; - return save(test, ast); - } - - const shouldOverWrite = process.env.OVERWRITE && !compareErrorsOnly; - - if (opts.throws) { - if (shouldOverWrite) { - const fn = path.dirname(test.expect.loc) + "/options.json"; - test.options = test.options || {}; - delete test.options.throws; - const contents = JSON.stringify(test.options, null, 2); - if (contents === "{}") { - fs.unlinkSync(fn); - } else { - fs.writeFileSync(fn, JSON.stringify(test.options, null, 2)); - } - test.expect.loc += "on"; - return save(test, ast); - } - - if (ast.errors && ast.errors.length) { - throw new Error( - `Expected non-recoverable error message: ${ - opts.throws - }. But instead parsing recovered from errors: ${JSON.stringify( - ast.errors, - null, - 2, - )}`, - ); - } else { - throw new Error( - `Expected error message: ${opts.throws}. But parsing succeeded without errors.`, - ); - } - } else if (compareErrorsOnly) { - const mis = misMatch(JSON.parse(test.expect.code).errors, ast.errors); - if (mis) { - throw new Error(mis); - } - } else { - const mis = misMatch(JSON.parse(test.expect.code), ast); - - if (mis) { - if (shouldOverWrite) { - return save(test, ast); - } - throw new Error(mis); - } } } -function serialize(value) { - if (typeof value === "bigint") { - return { - [serialized]: "BigInt", - value: value.toString(), - }; - } else if (value instanceof RegExp) { - return { - [serialized]: "RegExp", - source: value.source, - flags: value.flags, - }; - } else if (value instanceof Error) { - // Errors are serialized to a simple string, because are used frequently - return value.toString(); - } - return value; +function runAutogeneratedParseTests( + parse, + prefix, + task, + onlyCompareErrors = false, +) { + const { expect, options } = task; + const testFn = task.disabled ? it.skip : it; + const threw = options.throws ? options.throws : false; + const ast = expect.code ? deserialize(expect.code) : false; + const title = `${prefix}/${task.title}`; + const toStartPosition = ({ startLine = 1, startColumn = 0 }) => + `(${startLine}, ${startColumn})`; + + toFuzzedOptions(options) + .map(([adjust, options]) => ({ + ...task, + title: `${title} start = ${toStartPosition(options)}`, + adjust, + options, + source: task.actual.code, + expected: { threw, ast }, + })) + .forEach(test => + testFn(test.title, function () { + try { + runParseTest(parse, test, onlyCompareErrors); + } catch (err) { + if (!test.expect.code && !process.env.CI) { + const fn = path.dirname(task.expect.loc) + "/options.json"; + if (!fs.existsSync(fn)) { + test.options = task.options || {}; + test.options.throws = err.originalMessage || err.message; + fs.writeFileSync(fn, stringify(task.options, null, 2)); + } + } + /* + const fixturePath = `${path.relative( + rootPath, + fixturesPath, + )}/${name}/${task.actual.filename}`; + */ + throw new FixtureError(err, "fixturePath", task.actual.code); + } + }), + ); } -function ppJSON(v) { - if (typeof v === "bigint" || v instanceof Error || v instanceof RegExp) { - return ppJSON(serialize(v)); - } +const toJustErrors = result => ({ + threw: result.threw, + ast: result.ast && { errors: result.ast.errors }, +}); - if (v && typeof v === "object" && v[serialized]) { - switch (v[serialized]) { - case "BigInt": - return typeof BigInt === "undefined" ? "null" : v.value + "n"; - case "RegExp": - return `/${v.source}/${v.flags}`; - } - } else if (typeof v === "string" && /^[A-Z][a-z]+Error: /.test(v)) { - // Errors are serialized to a simple string, because are used frequently - return v; +function runParseTest(parse, test, onlyCompareErrors) { + const { adjust, expected, source, options } = test; + + if (expected.threw && expected.ast) { + throw Error( + "File expected.json exists although options specify throws. Remove expected.json.", + ); } - return JSON.stringify(v, (k, v) => serialize(v), 2); -} + const actual = parseWithRecovery(parse, source, options); + const difference = new Difference( + adjust, + onlyCompareErrors ? toJustErrors(expected) : expected, + onlyCompareErrors ? toJustErrors(actual) : actual, + ); -function addPath(str, pt) { - if (str.charAt(str.length - 1) === ")") { - return str.slice(0, str.length - 1) + "/" + pt + ")"; - } else { - return str + " (" + pt + ")"; + if (difference !== Difference.None) { + throw FixtureError2.fromDifference(difference); } } -function misMatch(exp, act) { - if ( - act instanceof RegExp || - act instanceof Error || - typeof act === "bigint" || - (exp && typeof exp === "object" && exp[serialized]) - ) { - const left = ppJSON(exp); - const right = ppJSON(act); - if (left !== right) return left + " !== " + right; - } else if (Array.isArray(exp)) { - if (!Array.isArray(act)) return ppJSON(exp) + " != " + ppJSON(act); - if (act.length != exp.length) { - return "array length mismatch " + exp.length + " != " + act.length; - } - for (let i = 0; i < act.length; ++i) { - const mis = misMatch(exp[i], act[i]); - if (mis) return addPath(mis, i); - } - } else if (!exp || !act || typeof exp != "object" || typeof act != "object") { - if (exp !== act && typeof exp != "function") { - return ppJSON(exp) + " !== " + ppJSON(act); - } - } else { - for (const prop of Object.keys(exp)) { - const mis = misMatch(exp[prop], act[prop]); - if (mis) return addPath(mis, prop); - } - - for (const prop of Object.keys(act)) { - if (typeof act[prop] === "function") { - continue; - } - - if (!(prop in exp) && act[prop] !== undefined) { - return `Did not expect a property '${prop}'`; - } - } +function parseWithRecovery(parse, source, options) { + try { + const ast = parse(source, { errorRecovery: true, ...options }); + + // Normalize the AST + // + // TODO: We should consider doing something more involved here as + // we may miss bugs where we put unexpected falsey objects in these + // properties. + if (ast.comments && !ast.comments.length) delete ast.comments; + if (ast.errors && !ast.errors.length) delete ast.errors; + else ast.errors = ast.errors.map(error => error + ""); + + return { threw: false, ast }; + } catch (error) { + return { threw: error.message, ast: false }; } } diff --git a/packages/babel-parser/test/helpers/to-fuzzed-options.js b/packages/babel-parser/test/helpers/to-fuzzed-options.js new file mode 100644 index 000000000000..0e307bf87353 --- /dev/null +++ b/packages/babel-parser/test/helpers/to-fuzzed-options.js @@ -0,0 +1,99 @@ +/* eslint-disable no-confusing-arrow */ + +const random = (min, max) => Math.floor(Math.random() * (max - min + 1) + min); +const clone = value => JSON.parse(JSON.stringify(value)); +// const dedupe = array => Object.values(Object.fromEntries(array)); + +const toDescriptorAssignedObject = (delta, object) => + delta.reduce( + (object, [key, descriptor]) => ( + !descriptor || descriptor.delete + ? delete object[key] + : Object.defineProperty(object, key, descriptor), + object + ), + clone(object), + ); + +const toAdjust = adjustments => + !adjustments || Object.keys(adjustments).length === 0 + ? null + : Object.assign( + (value, key, parent) => + key in adjustments ? adjustments[key](value, key, parent) : value, + adjustments, + ); + +const toAdjustedErrorMessage = adjust => message => + message && + message.replace(/\((\d+):(\d+)\)$/, function (_, line, column) { + const loc = { line: parseInt(line, 10), column: parseInt(column, 10) }; + return `(${adjust( + loc.line, + "line", + loc, + )}:${adjust(loc.column, "column", loc)})`; + }); + +export default function toFuzzedOptions(options) { + const { startLine = 1, startColumn = 0 } = options; + + // If the test supplies its own position, then make sure we choose + // a different position. Also, make sure we stay wihtin the "reasonable" + // bounds in case the test is testing negative startLine or startColumn + // for example. + const randomLine = Math.max(2, random(startLine + 1, 1000)); + const randomColumn = Math.max(1, random(startColumn + 1, 100)); + + // Now assemble our deltas... + const fuzzedOptions = [ + [false, false], + [1, 0], + [1, randomColumn], + [randomLine, 0], + [randomLine, randomColumn], + [randomLine, false], + [false, randomColumn], + ] + .map(([line, column]) => [ + ["startLine", line !== false && { enumerable: true, value: line }], + ["startColumn", column !== false && { enumerable: true, value: column }], + ]) + .map(delta => toDescriptorAssignedObject(delta, options)); + + // Make sure to include the original options in our set as well if the user + // is wanting to test a specific start position. + const totalOptions = + startLine !== 1 || startColumn !== 0 + ? [options, ...fuzzedOptions] + : fuzzedOptions; + + // The last step is to create our fuzzing function for traversing the resulting AST. + // This allows us to efficiently try these different options without having to modify + // the expected results. + return totalOptions + .map(options => [options, options.startLine || 1, options.startColumn || 0]) + .map(([options, fStartLine, fStartColumn]) => [ + toAdjust({ + ...(startLine !== fStartLine && { + line: line => line - startLine + fStartLine, + }), + ...(startColumn !== fStartColumn && { + column: (column, _, { line }) => + line !== startLine ? column : column - startColumn + fStartColumn, + }), + }), + options, + ]) + .map(([adjust, options]) => [ + toAdjust({ + ...adjust, + ...(adjust && { threw: toAdjustedErrorMessage(adjust) }), + ...(adjust && { + errors: errors => + errors && errors.map(toAdjustedErrorMessage(adjust)), + }), + }), + options, + ]); +} diff --git a/packages/babel-parser/test/index.js b/packages/babel-parser/test/index.js index 885e1948eb7d..87079c5d5bcd 100644 --- a/packages/babel-parser/test/index.js +++ b/packages/babel-parser/test/index.js @@ -1,5 +1,5 @@ import path from "path"; -import { runFixtureTests } from "./helpers/runFixtureTests.js"; +import runFixtureTests from "./helpers/runFixtureTests.js"; import { parse } from "../lib/index.js"; import { fileURLToPath } from "url"; From aee94862dd7951b31e123080489cee5ad6f8bff2 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Wed, 19 Jan 2022 16:17:53 -0800 Subject: [PATCH 02/34] Only deserialize JSON files that are serialized. Reviewed by @tolmasky. --- packages/babel-helper-fixtures/src/index.ts | 1 + .../{output.json => output.serialized.json} | 0 .../{output.json => output.serialized.json} | 0 .../test/helpers/runFixtureTests.js | 33 ++++++++++++------- 4 files changed, 23 insertions(+), 11 deletions(-) rename packages/babel-parser/test/fixtures/estree/bigInt/basic/{output.json => output.serialized.json} (100%) rename packages/babel-parser/test/fixtures/estree/literal/regexp/{output.json => output.serialized.json} (100%) diff --git a/packages/babel-helper-fixtures/src/index.ts b/packages/babel-helper-fixtures/src/index.ts index 7e9770b49c2a..ee3a463ba79b 100644 --- a/packages/babel-helper-fixtures/src/index.ts +++ b/packages/babel-helper-fixtures/src/index.ts @@ -105,6 +105,7 @@ function pushTask(taskName, taskDir, suite, suiteName) { const expectLoc = findFile(taskDir + "/output", true /* allowJSON */) || + findFile(`${taskDir}/output.serialized`, true) || taskDir + "/output.js"; const stdoutLoc = taskDir + "/stdout.txt"; const stderrLoc = taskDir + "/stderr.txt"; diff --git a/packages/babel-parser/test/fixtures/estree/bigInt/basic/output.json b/packages/babel-parser/test/fixtures/estree/bigInt/basic/output.serialized.json similarity index 100% rename from packages/babel-parser/test/fixtures/estree/bigInt/basic/output.json rename to packages/babel-parser/test/fixtures/estree/bigInt/basic/output.serialized.json diff --git a/packages/babel-parser/test/fixtures/estree/literal/regexp/output.json b/packages/babel-parser/test/fixtures/estree/literal/regexp/output.serialized.json similarity index 100% rename from packages/babel-parser/test/fixtures/estree/literal/regexp/output.json rename to packages/babel-parser/test/fixtures/estree/literal/regexp/output.serialized.json diff --git a/packages/babel-parser/test/helpers/runFixtureTests.js b/packages/babel-parser/test/helpers/runFixtureTests.js index 9fbdc20b0497..9dcf124acdd0 100644 --- a/packages/babel-parser/test/helpers/runFixtureTests.js +++ b/packages/babel-parser/test/helpers/runFixtureTests.js @@ -8,34 +8,43 @@ import FixtureError2 from "./fixture-error.js"; import toFuzzedOptions from "./to-fuzzed-options.js"; const { parse: JSONParse, stringify } = JSON; +const isSerialized = filename => /\.serialized\.json$/.test(filename); // We've only serialized one BigInt in the entire test suite: // -// packages/babel-parser/test/fixtures/estree/basic/bigint/output.json +// packages/babel-parser/test/fixtures/estree/basic/bigint/output.serialized.json // // This is because only estree actually includes the BigInt value in the Literal -// node. If the JS environemnt doesn['t support bigint, then estree will just +// node. If the JS environemnt doesn't support bigint, then estree will just // use null for the value. We also happen to just throw the AST information away // with estree tests, so in the event that we're running on an older version of // Node that doesn't support bigint, it is safe to deserialize to null. const toBigInt = global.BigInt || (() => null); +const SerializationKey = "$$ babel internal serialized type"; + /* eslint-disable no-confusing-arrow */ -const deserialize = string => - JSONParse(string, (key, value) => - key !== "value" || !value || typeof value !== "object" || !value[serialized] - ? value - : value[serialized] === "RegExp" - ? new RegExp(value.source, value.flags) - : toBigInt(value.value), +const deserialize = (serialized, string) => + JSONParse( + string, + serialized && + ((key, value) => + key !== "value" || + !value || + typeof value !== "object" || + !value[SerializationKey] + ? value + : value[SerializationKey] === "RegExp" + ? new RegExp(value.source, value.flags) + : toBigInt(value.value)), ); + /* const rootPath = path.join( path.dirname(fileURLToPath(import.meta.url)), "../../../..", ); */ -const serialized = "$$ babel internal serialized type"; class FixtureError extends Error { constructor(previousError, fixturePath, code) { @@ -101,7 +110,9 @@ function runAutogeneratedParseTests( const { expect, options } = task; const testFn = task.disabled ? it.skip : it; const threw = options.throws ? options.throws : false; - const ast = expect.code ? deserialize(expect.code) : false; + const ast = expect.code + ? deserialize(isSerialized(expect.loc), expect.code) + : false; const title = `${prefix}/${task.title}`; const toStartPosition = ({ startLine = 1, startColumn = 0 }) => `(${startLine}, ${startColumn})`; From 4737700d292fef26943fd8c3032d00a77fc60cef Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Fri, 21 Jan 2022 08:01:51 -0800 Subject: [PATCH 03/34] Don't fuzz test in node 8, since for whatever reason Jest 23 on node 8 takes forever after a certain number of tests, even if those tests are empty. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/to-fuzzed-options.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/babel-parser/test/helpers/to-fuzzed-options.js b/packages/babel-parser/test/helpers/to-fuzzed-options.js index 0e307bf87353..64d9fa2d87a8 100644 --- a/packages/babel-parser/test/helpers/to-fuzzed-options.js +++ b/packages/babel-parser/test/helpers/to-fuzzed-options.js @@ -36,6 +36,13 @@ const toAdjustedErrorMessage = adjust => message => }); export default function toFuzzedOptions(options) { + // Don't fuzz test in node 8, since for whatever reason Jest 23 on node 8 + // takes forever after a certain number of tests, even if those tests are + // empty. + if (/v8\./.test(process.version)) { + return [[false, options]]; + } + const { startLine = 1, startColumn = 0 } = options; // If the test supplies its own position, then make sure we choose From 4bf3933eb431b1f2861c2aeb6c0f823c1aca2ca5 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Fri, 21 Jan 2022 08:17:24 -0800 Subject: [PATCH 04/34] Rename runFixtureTests.js to run-fixture-tests.js. Reviewed by @tolmasky. --- packages/babel-parser/test/attachComment-false.js | 2 +- packages/babel-parser/test/estree-throws.js | 2 +- packages/babel-parser/test/expressions.js | 2 +- .../test/helpers/{runFixtureTests.js => run-fixture-tests.js} | 0 packages/babel-parser/test/index.js | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename packages/babel-parser/test/helpers/{runFixtureTests.js => run-fixture-tests.js} (100%) diff --git a/packages/babel-parser/test/attachComment-false.js b/packages/babel-parser/test/attachComment-false.js index ea61f229030e..c2ecf7a635a3 100644 --- a/packages/babel-parser/test/attachComment-false.js +++ b/packages/babel-parser/test/attachComment-false.js @@ -1,5 +1,5 @@ import path from "path"; -import runFixtureTests from "./helpers/runFixtureTests.js"; +import runFixtureTests from "./helpers/run-fixture-tests.js"; import { parseExpression } from "../lib/index.js"; import { fileURLToPath } from "url"; diff --git a/packages/babel-parser/test/estree-throws.js b/packages/babel-parser/test/estree-throws.js index 9e70e9cf365f..ee5cfa062928 100644 --- a/packages/babel-parser/test/estree-throws.js +++ b/packages/babel-parser/test/estree-throws.js @@ -1,5 +1,5 @@ import path from "path"; -import runFixtureTests from "./helpers/runFixtureTests.js"; +import runFixtureTests from "./helpers/run-fixture-tests.js"; import { parse } from "../lib/index.js"; import { fileURLToPath } from "url"; diff --git a/packages/babel-parser/test/expressions.js b/packages/babel-parser/test/expressions.js index c1dfd6d32407..600b8085affd 100644 --- a/packages/babel-parser/test/expressions.js +++ b/packages/babel-parser/test/expressions.js @@ -1,5 +1,5 @@ import path from "path"; -import runFixtureTests from "./helpers/runFixtureTests.js"; +import runFixtureTests from "./helpers/run-fixture-tests.js"; import { parseExpression } from "../lib/index.js"; import { fileURLToPath } from "url"; diff --git a/packages/babel-parser/test/helpers/runFixtureTests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js similarity index 100% rename from packages/babel-parser/test/helpers/runFixtureTests.js rename to packages/babel-parser/test/helpers/run-fixture-tests.js diff --git a/packages/babel-parser/test/index.js b/packages/babel-parser/test/index.js index 87079c5d5bcd..70e0193729a9 100644 --- a/packages/babel-parser/test/index.js +++ b/packages/babel-parser/test/index.js @@ -1,5 +1,5 @@ import path from "path"; -import runFixtureTests from "./helpers/runFixtureTests.js"; +import runFixtureTests from "./helpers/run-fixture-tests.js"; import { parse } from "../lib/index.js"; import { fileURLToPath } from "url"; From 60b039e39f2cad90ae190af5824b6da04079a355 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Sat, 22 Jan 2022 18:43:26 -0800 Subject: [PATCH 05/34] Fix for showing code snippet again. Reviewed by @tolmasky. --- .../babel-parser/test/helpers/difference.js | 8 +- .../test/helpers/fixture-error.js | 25 +++- .../test/helpers/run-fixture-tests.js | 118 ++++-------------- .../test/helpers/serialization.js | 37 ++++++ .../helpers/to-contextual-syntax-error.js | 34 +++++ .../test/helpers/to-fuzzed-options.js | 62 +++++---- test/jest-light-runner/src/worker-runner.js | 3 +- 7 files changed, 161 insertions(+), 126 deletions(-) create mode 100644 packages/babel-parser/test/helpers/serialization.js create mode 100644 packages/babel-parser/test/helpers/to-contextual-syntax-error.js diff --git a/packages/babel-parser/test/helpers/difference.js b/packages/babel-parser/test/helpers/difference.js index daa54411e09e..a0b59a7ef42f 100644 --- a/packages/babel-parser/test/helpers/difference.js +++ b/packages/babel-parser/test/helpers/difference.js @@ -33,6 +33,8 @@ const toType = value => ? "Array" : value instanceof RegExp ? "RegExp" + : value instanceof Error + ? "Error" : "Object"; function compare(adjust, expected, actual) { @@ -57,6 +59,10 @@ function compare(adjust, expected, actual) { return false; } + if (typeActual === "Error") { + return compare(adjust, { message: expected.message }, { message: actual.message }); + } + if (typeActual !== "Object" && typeActual !== "Array") { return { discrepancy: "value", expected, actual }; } @@ -91,7 +97,7 @@ function compare(adjust, expected, actual) { } const original = expected[key]; - const adjusted = adjust ? adjust(original, key, expected) : original; + const adjusted = adjust ? adjust(adjust, original, key, expected) : original; const difference = compare(adjust, adjusted, actual[key]); if (difference) { return [key, difference]; diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index 53cb962444b8..a463d4acb73f 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -1,3 +1,4 @@ +import { inspect } from "util"; import "./polyfill.js"; const { defineProperty, entries, fromEntries } = Object; @@ -35,7 +36,7 @@ Object.assign( DifferentAST: ({ message }) => message, UnexpectedError: ({ actual }) => - `Encountered unexpected non-recoverable message:\n\n${actual}`, + `Encountered unexpected unrecoverable error.\n`, UnexpectedSuccess: ({ expected }) => `Expected non-recoverable error message:\n\n${expected}\n\n` + @@ -56,9 +57,25 @@ Object.assign( class extends FixtureError { constructor(difference, ...args) { super(toMessage(difference, ...args), difference); + + this.cause = + (!!(difference.actual instanceof Error) && + difference.actual.context) || + difference.actual; + } + + // Don't show the stack of FixtureErrors, it's irrelevant. + // Instead, show the cause, if present. + [inspect.custom](depth, options) { + return `${this.message.replace(/\.\n$/, ":\n")}${ + this.cause + ? `\n${inspect(this.cause, options)}`.replace(/\n/g, "\n ") + : "" + }`; } - }, + } ), - ], - ), + ] + ) ); + diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 9dcf124acdd0..533405599736 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -1,84 +1,14 @@ import { multiple as getFixtures } from "@babel/helper-fixtures"; import { codeFrameColumns } from "@babel/code-frame"; import fs from "fs"; -import path from "path"; -// import { fileURLToPath } from "url"; +import { dirname } from "path"; import Difference from "./difference.js"; -import FixtureError2 from "./fixture-error.js"; +import FixtureError from "./fixture-error.js"; import toFuzzedOptions from "./to-fuzzed-options.js"; +import { deserialize } from "./serialization.js" +import toContextualSyntaxError from "./to-contextual-syntax-error.js"; -const { parse: JSONParse, stringify } = JSON; -const isSerialized = filename => /\.serialized\.json$/.test(filename); - -// We've only serialized one BigInt in the entire test suite: -// -// packages/babel-parser/test/fixtures/estree/basic/bigint/output.serialized.json -// -// This is because only estree actually includes the BigInt value in the Literal -// node. If the JS environemnt doesn't support bigint, then estree will just -// use null for the value. We also happen to just throw the AST information away -// with estree tests, so in the event that we're running on an older version of -// Node that doesn't support bigint, it is safe to deserialize to null. -const toBigInt = global.BigInt || (() => null); - -const SerializationKey = "$$ babel internal serialized type"; - -/* eslint-disable no-confusing-arrow */ -const deserialize = (serialized, string) => - JSONParse( - string, - serialized && - ((key, value) => - key !== "value" || - !value || - typeof value !== "object" || - !value[SerializationKey] - ? value - : value[SerializationKey] === "RegExp" - ? new RegExp(value.source, value.flags) - : toBigInt(value.value)), - ); - -/* -const rootPath = path.join( - path.dirname(fileURLToPath(import.meta.url)), - "../../../..", -); -*/ - -class FixtureError extends Error { - constructor(previousError, fixturePath, code) { - super(previousError.message); - const messageLines = (previousError.message.match(/\n/g) || []).length + 1; - - let fixtureStackFrame = ""; - if (previousError.loc) { - fixtureStackFrame = - codeFrameColumns( - code, - { - start: { - line: previousError.loc.line, - column: previousError.loc.column + 1, - }, - }, - { highlightCode: true }, - ) + - "\n" + - `at fixture (${fixturePath}:${previousError.loc.line}:${ - previousError.loc.column + 1 - })\n`; - } - - this.stack = - previousError.constructor.name + - ": " + - previousError.message + - "\n" + - fixtureStackFrame + - previousError.stack.split("\n").slice(messageLines).join("\n"); - } -} +const { stringify } = JSON; export default function runFixtureTests( fixturesPath, @@ -109,10 +39,9 @@ function runAutogeneratedParseTests( ) { const { expect, options } = task; const testFn = task.disabled ? it.skip : it; - const threw = options.throws ? options.throws : false; - const ast = expect.code - ? deserialize(isSerialized(expect.loc), expect.code) - : false; + const threw = options.throws ? toSyntaxError(options.throws) : false; + const ast = expect.code ? deserialize(expect.loc, expect.code) : false; + const errors = !!ast && !!ast.errors && ast.errors.map(toSyntaxError); const title = `${prefix}/${task.title}`; const toStartPosition = ({ startLine = 1, startColumn = 0 }) => `(${startLine}, ${startColumn})`; @@ -122,9 +51,9 @@ function runAutogeneratedParseTests( ...task, title: `${title} start = ${toStartPosition(options)}`, adjust, - options, + options: { ...options, sourceFilename: task.actual.loc }, source: task.actual.code, - expected: { threw, ast }, + expected: { threw, ast: errors ? { ...ast, errors } : ast } })) .forEach(test => testFn(test.title, function () { @@ -132,25 +61,22 @@ function runAutogeneratedParseTests( runParseTest(parse, test, onlyCompareErrors); } catch (err) { if (!test.expect.code && !process.env.CI) { - const fn = path.dirname(task.expect.loc) + "/options.json"; + const fn = dirname(task.expect.loc) + "/options.json"; if (!fs.existsSync(fn)) { test.options = task.options || {}; test.options.throws = err.originalMessage || err.message; fs.writeFileSync(fn, stringify(task.options, null, 2)); } } - /* - const fixturePath = `${path.relative( - rootPath, - fixturesPath, - )}/${name}/${task.actual.filename}`; - */ - throw new FixtureError(err, "fixturePath", task.actual.code); + throw err; } }), ); } +const toSyntaxError = message => + SyntaxError(message.replace(/^SyntaxError:\s/, "")); + const toJustErrors = result => ({ threw: result.threw, ast: result.ast && { errors: result.ast.errors }, @@ -173,11 +99,11 @@ function runParseTest(parse, test, onlyCompareErrors) { ); if (difference !== Difference.None) { - throw FixtureError2.fromDifference(difference); + throw FixtureError.fromDifference(difference); } } -function parseWithRecovery(parse, source, options) { +function parseWithRecovery(parse, source, { sourceFilename, ...options }) { try { const ast = parse(source, { errorRecovery: true, ...options }); @@ -188,10 +114,16 @@ function parseWithRecovery(parse, source, options) { // properties. if (ast.comments && !ast.comments.length) delete ast.comments; if (ast.errors && !ast.errors.length) delete ast.errors; - else ast.errors = ast.errors.map(error => error + ""); + else + ast.errors = ast.errors.map((error) => + toContextualSyntaxError(error, source, options) + ); return { threw: false, ast }; } catch (error) { - return { threw: error.message, ast: false }; + return { + threw: toContextualSyntaxError(error, source, options), + ast: false, + }; } } diff --git a/packages/babel-parser/test/helpers/serialization.js b/packages/babel-parser/test/helpers/serialization.js new file mode 100644 index 000000000000..006c4ffa1f36 --- /dev/null +++ b/packages/babel-parser/test/helpers/serialization.js @@ -0,0 +1,37 @@ +const { parse: JSONParse } = JSON; + +// We give JSON files that needed our special serialization the extension +// ".serialized.json" instead of just ".json" so that we can only use our +// deserialization function in those cases (which is slower), and in all +// other instances just rely on normal JSON.parse with no deserialization +// function. +const isSerialized = filename => /\.serialized\.json$/.test(filename); + +// We've only serialized one BigInt in the entire test suite: +// +// packages/babel-parser/test/fixtures/estree/basic/bigint/output.serialized.json +// +// This is because only estree actually includes the BigInt value in the Literal +// node. If the JS environemnt doesn't support bigint, then estree will just +// use null for the value. We also happen to just throw the AST information away +// with estree tests, so in the event that we're running on an older version of +// Node that doesn't support bigint, it is safe to deserialize to null. +const toBigInt = global.BigInt || (() => null); + +const SerializationKey = "$$ babel internal serialized type"; + +/* eslint-disable no-confusing-arrow */ +export const deserialize = (filename, string) => + JSONParse( + string, + isSerialized(filename) && + ((key, value) => + key !== "value" || + !value || + typeof value !== "object" || + !value[SerializationKey] + ? value + : value[SerializationKey] === "RegExp" + ? new RegExp(value.source, value.flags) + : toBigInt(value.value)), + ); diff --git a/packages/babel-parser/test/helpers/to-contextual-syntax-error.js b/packages/babel-parser/test/helpers/to-contextual-syntax-error.js new file mode 100644 index 000000000000..94c4c69e9cdc --- /dev/null +++ b/packages/babel-parser/test/helpers/to-contextual-syntax-error.js @@ -0,0 +1,34 @@ +import { codeFrameColumns } from "@babel/code-frame"; +import { runInThisContext } from "vm"; + +export default function toContextualSyntaxError(error, source, options) { + if (!(error instanceof SyntaxError)) return error; + + const { startLine = 1, startColumn = 0, sourceFilename } = options || {}; + const line = error.loc.line - startLine + 1; + const column = + 1 + (line === 1 ? error.loc.column - startColumn : error.loc.column); + const frame = codeFrameColumns( + source, + { start: { line, column } }, + { highlightCode: true } + ); + + // Limit this stack trace to 1. Everything after that is just stuff from the + // test. + const originalStackTraceLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 1; + + Object.defineProperty(error, "context", { + value: (error.context = runInThisContext("(f => f())", { + filename: sourceFilename, + })(() => SyntaxError(`${error.message}\n${frame}`))), + enumerable: false + }); + + Error.stackTraceLimit = originalStackTraceLimit; + + error.context.cause = error; + + return error; +} diff --git a/packages/babel-parser/test/helpers/to-fuzzed-options.js b/packages/babel-parser/test/helpers/to-fuzzed-options.js index 64d9fa2d87a8..6c263d31f154 100644 --- a/packages/babel-parser/test/helpers/to-fuzzed-options.js +++ b/packages/babel-parser/test/helpers/to-fuzzed-options.js @@ -19,28 +19,46 @@ const toAdjust = adjustments => !adjustments || Object.keys(adjustments).length === 0 ? null : Object.assign( - (value, key, parent) => - key in adjustments ? adjustments[key](value, key, parent) : value, - adjustments, + (adjust, value, key, parent) => + key in adjustments + ? adjustments[key](adjust, value, key, parent) + : value, + adjustments ); -const toAdjustedErrorMessage = adjust => message => - message && - message.replace(/\((\d+):(\d+)\)$/, function (_, line, column) { - const loc = { line: parseInt(line, 10), column: parseInt(column, 10) }; - return `(${adjust( - loc.line, - "line", - loc, - )}:${adjust(loc.column, "column", loc)})`; - }); +const SyntaxErrorMessageRegExp = /\((\d+):(\d+)\)$/; +const toAdjustedSyntaxError = (adjust, error) => + error && SyntaxErrorMessageRegExp.test(error.message) + ? SyntaxError( + error.message.replace(/\((\d+):(\d+)\)$/, function (_, line, column) { + const loc = { + line: parseInt(line, 10), + column: parseInt(column, 10), + }; + return `(${adjust( + adjust, + loc.line, + "line", + loc + )}:${adjust(adjust, loc.column, "column", loc)})`; + }) + ) + : error; + +const DefaultAdjust = toAdjust({ + filename: () => void 0, + threw: (adjust, error) => + error && toAdjustedSyntaxError(adjust, error), + errors: (adjust, errors) => + errors && errors.map(error => toAdjustedSyntaxError(adjust, error)), +}); export default function toFuzzedOptions(options) { // Don't fuzz test in node 8, since for whatever reason Jest 23 on node 8 // takes forever after a certain number of tests, even if those tests are // empty. if (/v8\./.test(process.version)) { - return [[false, options]]; + return [[DefaultAdjust, options]]; } const { startLine = 1, startColumn = 0 } = options; @@ -82,25 +100,15 @@ export default function toFuzzedOptions(options) { .map(options => [options, options.startLine || 1, options.startColumn || 0]) .map(([options, fStartLine, fStartColumn]) => [ toAdjust({ + ...DefaultAdjust, ...(startLine !== fStartLine && { - line: line => line - startLine + fStartLine, + line: (_, line) => line - startLine + fStartLine, }), ...(startColumn !== fStartColumn && { - column: (column, _, { line }) => + column: (_, column, __, { line }) => line !== startLine ? column : column - startColumn + fStartColumn, }), }), options, - ]) - .map(([adjust, options]) => [ - toAdjust({ - ...adjust, - ...(adjust && { threw: toAdjustedErrorMessage(adjust) }), - ...(adjust && { - errors: errors => - errors && errors.map(toAdjustedErrorMessage(adjust)), - }), - }), - options, ]); } diff --git a/test/jest-light-runner/src/worker-runner.js b/test/jest-light-runner/src/worker-runner.js index 03474feea893..554ca60370d9 100644 --- a/test/jest-light-runner/src/worker-runner.js +++ b/test/jest-light-runner/src/worker-runner.js @@ -4,6 +4,7 @@ import { performance } from "perf_hooks"; import snapshot from "jest-snapshot"; import expect from "expect"; import * as circus from "jest-circus"; +import { inspect } from "util" import "./global-setup.js"; @@ -248,7 +249,7 @@ function failureToString(test) { return ( test.ancestors.concat(test.title).join(" > ") + "\n" + - test.errors.map(e => e.toString().replace(/^/gm, "\t")).join("\n") + + test.errors.map(error => inspect(error).replace(/^/gm, " ")).join("\n") + "\n" ); } From 17c1093c1601e63deb8bc2633efec531f9fdb5d8 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Sat, 22 Jan 2022 20:23:03 -0800 Subject: [PATCH 06/34] Fix some linter errors. Reviewed by @tolmasky. --- .../babel-parser/test/helpers/difference.js | 11 +++++++-- .../test/helpers/fixture-error.js | 10 ++++---- .../test/helpers/run-fixture-tests.js | 23 ++++++++++--------- .../helpers/to-contextual-syntax-error.js | 11 ++++++--- .../test/helpers/to-fuzzed-options.js | 10 ++++---- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/packages/babel-parser/test/helpers/difference.js b/packages/babel-parser/test/helpers/difference.js index a0b59a7ef42f..0a279eebcc91 100644 --- a/packages/babel-parser/test/helpers/difference.js +++ b/packages/babel-parser/test/helpers/difference.js @@ -60,7 +60,11 @@ function compare(adjust, expected, actual) { } if (typeActual === "Error") { - return compare(adjust, { message: expected.message }, { message: actual.message }); + return compare( + adjust, + { message: expected.message }, + { message: actual.message }, + ); } if (typeActual !== "Object" && typeActual !== "Array") { @@ -97,8 +101,11 @@ function compare(adjust, expected, actual) { } const original = expected[key]; - const adjusted = adjust ? adjust(adjust, original, key, expected) : original; + const adjusted = adjust + ? adjust(adjust, original, key, expected) + : original; const difference = compare(adjust, adjusted, actual[key]); + if (difference) { return [key, difference]; } diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index a463d4acb73f..2e8ce61ab939 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -35,8 +35,7 @@ Object.assign( DifferentAST: ({ message }) => message, - UnexpectedError: ({ actual }) => - `Encountered unexpected unrecoverable error.\n`, + UnexpectedError: () => `Encountered unexpected unrecoverable error.\n`, UnexpectedSuccess: ({ expected }) => `Expected non-recoverable error message:\n\n${expected}\n\n` + @@ -73,9 +72,8 @@ Object.assign( : "" }`; } - } + }, ), - ] - ) + ], + ), ); - diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 533405599736..ad71350f3a20 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -1,11 +1,10 @@ import { multiple as getFixtures } from "@babel/helper-fixtures"; -import { codeFrameColumns } from "@babel/code-frame"; import fs from "fs"; import { dirname } from "path"; import Difference from "./difference.js"; import FixtureError from "./fixture-error.js"; import toFuzzedOptions from "./to-fuzzed-options.js"; -import { deserialize } from "./serialization.js" +import { deserialize } from "./serialization.js"; import toContextualSyntaxError from "./to-contextual-syntax-error.js"; const { stringify } = JSON; @@ -51,9 +50,10 @@ function runAutogeneratedParseTests( ...task, title: `${title} start = ${toStartPosition(options)}`, adjust, - options: { ...options, sourceFilename: task.actual.loc }, + filename: task.actual.loc, + options, source: task.actual.code, - expected: { threw, ast: errors ? { ...ast, errors } : ast } + expected: { threw, ast: errors ? { ...ast, errors } : ast }, })) .forEach(test => testFn(test.title, function () { @@ -83,7 +83,7 @@ const toJustErrors = result => ({ }); function runParseTest(parse, test, onlyCompareErrors) { - const { adjust, expected, source, options } = test; + const { adjust, expected, source, filename, options } = test; if (expected.threw && expected.ast) { throw Error( @@ -91,7 +91,7 @@ function runParseTest(parse, test, onlyCompareErrors) { ); } - const actual = parseWithRecovery(parse, source, options); + const actual = parseWithRecovery(parse, source, filename, options); const difference = new Difference( adjust, onlyCompareErrors ? toJustErrors(expected) : expected, @@ -103,7 +103,7 @@ function runParseTest(parse, test, onlyCompareErrors) { } } -function parseWithRecovery(parse, source, { sourceFilename, ...options }) { +function parseWithRecovery(parse, source, filename, options) { try { const ast = parse(source, { errorRecovery: true, ...options }); @@ -114,15 +114,16 @@ function parseWithRecovery(parse, source, { sourceFilename, ...options }) { // properties. if (ast.comments && !ast.comments.length) delete ast.comments; if (ast.errors && !ast.errors.length) delete ast.errors; - else - ast.errors = ast.errors.map((error) => - toContextualSyntaxError(error, source, options) + else { + ast.errors = ast.errors.map(error => + toContextualSyntaxError(error, source, filename, options), ); + } return { threw: false, ast }; } catch (error) { return { - threw: toContextualSyntaxError(error, source, options), + threw: toContextualSyntaxError(error, source, filename, options), ast: false, }; } diff --git a/packages/babel-parser/test/helpers/to-contextual-syntax-error.js b/packages/babel-parser/test/helpers/to-contextual-syntax-error.js index 94c4c69e9cdc..96039375b3e4 100644 --- a/packages/babel-parser/test/helpers/to-contextual-syntax-error.js +++ b/packages/babel-parser/test/helpers/to-contextual-syntax-error.js @@ -1,7 +1,12 @@ import { codeFrameColumns } from "@babel/code-frame"; import { runInThisContext } from "vm"; -export default function toContextualSyntaxError(error, source, options) { +export default function toContextualSyntaxError( + error, + source, + filename, + options, +) { if (!(error instanceof SyntaxError)) return error; const { startLine = 1, startColumn = 0, sourceFilename } = options || {}; @@ -11,7 +16,7 @@ export default function toContextualSyntaxError(error, source, options) { const frame = codeFrameColumns( source, { start: { line, column } }, - { highlightCode: true } + { highlightCode: true }, ); // Limit this stack trace to 1. Everything after that is just stuff from the @@ -23,7 +28,7 @@ export default function toContextualSyntaxError(error, source, options) { value: (error.context = runInThisContext("(f => f())", { filename: sourceFilename, })(() => SyntaxError(`${error.message}\n${frame}`))), - enumerable: false + enumerable: false, }); Error.stackTraceLimit = originalStackTraceLimit; diff --git a/packages/babel-parser/test/helpers/to-fuzzed-options.js b/packages/babel-parser/test/helpers/to-fuzzed-options.js index 6c263d31f154..30170e690a33 100644 --- a/packages/babel-parser/test/helpers/to-fuzzed-options.js +++ b/packages/babel-parser/test/helpers/to-fuzzed-options.js @@ -23,7 +23,7 @@ const toAdjust = adjustments => key in adjustments ? adjustments[key](adjust, value, key, parent) : value, - adjustments + adjustments, ); const SyntaxErrorMessageRegExp = /\((\d+):(\d+)\)$/; @@ -39,16 +39,14 @@ const toAdjustedSyntaxError = (adjust, error) => adjust, loc.line, "line", - loc + loc, )}:${adjust(adjust, loc.column, "column", loc)})`; - }) + }), ) : error; const DefaultAdjust = toAdjust({ - filename: () => void 0, - threw: (adjust, error) => - error && toAdjustedSyntaxError(adjust, error), + threw: (adjust, error) => error && toAdjustedSyntaxError(adjust, error), errors: (adjust, errors) => errors && errors.map(error => toAdjustedSyntaxError(adjust, error)), }); From c140ba83e8e87c9cf0a84d500e5e673fb14a25d8 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Sun, 23 Jan 2022 07:44:47 -0800 Subject: [PATCH 07/34] Fix stack traces and only generate the context error when we need it. Reviewed by @tolmasky. --- .../helpers/to-contextual-syntax-error.js | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/babel-parser/test/helpers/to-contextual-syntax-error.js b/packages/babel-parser/test/helpers/to-contextual-syntax-error.js index 96039375b3e4..ba1607280d3d 100644 --- a/packages/babel-parser/test/helpers/to-contextual-syntax-error.js +++ b/packages/babel-parser/test/helpers/to-contextual-syntax-error.js @@ -9,7 +9,7 @@ export default function toContextualSyntaxError( ) { if (!(error instanceof SyntaxError)) return error; - const { startLine = 1, startColumn = 0, sourceFilename } = options || {}; + const { startLine = 1, startColumn = 0 } = options || {}; const line = error.loc.line - startLine + 1; const column = 1 + (line === 1 ? error.loc.column - startColumn : error.loc.column); @@ -21,19 +21,20 @@ export default function toContextualSyntaxError( // Limit this stack trace to 1. Everything after that is just stuff from the // test. - const originalStackTraceLimit = Error.stackTraceLimit; - Error.stackTraceLimit = 1; - Object.defineProperty(error, "context", { - value: (error.context = runInThisContext("(f => f())", { - filename: sourceFilename, - })(() => SyntaxError(`${error.message}\n${frame}`))), - enumerable: false, - }); + get() { + const message = JSON.stringify(`${error.message}\n${frame}`); + const originalStackTraceLimit = Error.stackTraceLimit; + + Error.stackTraceLimit = 1; - Error.stackTraceLimit = originalStackTraceLimit; + const context = runInThisContext(`SyntaxError(${message})`, { filename }); - error.context.cause = error; + Error.stackTraceLimit = originalStackTraceLimit; + + return Object.assign(context, { cause: this }); + }, + }); return error; } From e10465ae9b310c2489603ffee0a4ba9c19c22752 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Sun, 23 Jan 2022 10:55:02 -0800 Subject: [PATCH 08/34] Create errors in deserialization step instead of using adjust to convert them. Reviewed by @tolmasky. --- .../test/helpers/run-fixture-tests.js | 12 ++-- .../test/helpers/serialization.js | 62 +++++++++++++++---- .../test/helpers/to-fuzzed-options.js | 20 +++--- 3 files changed, 65 insertions(+), 29 deletions(-) diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index ad71350f3a20..74fb9b8ed608 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -38,9 +38,8 @@ function runAutogeneratedParseTests( ) { const { expect, options } = task; const testFn = task.disabled ? it.skip : it; - const threw = options.throws ? toSyntaxError(options.throws) : false; - const ast = expect.code ? deserialize(expect.loc, expect.code) : false; - const errors = !!ast && !!ast.errors && ast.errors.map(toSyntaxError); + + const expected = deserialize(expect.loc, options, expect.code); const title = `${prefix}/${task.title}`; const toStartPosition = ({ startLine = 1, startColumn = 0 }) => `(${startLine}, ${startColumn})`; @@ -50,10 +49,10 @@ function runAutogeneratedParseTests( ...task, title: `${title} start = ${toStartPosition(options)}`, adjust, - filename: task.actual.loc, options, + expected, + filename: task.actual.loc, source: task.actual.code, - expected: { threw, ast: errors ? { ...ast, errors } : ast }, })) .forEach(test => testFn(test.title, function () { @@ -74,9 +73,6 @@ function runAutogeneratedParseTests( ); } -const toSyntaxError = message => - SyntaxError(message.replace(/^SyntaxError:\s/, "")); - const toJustErrors = result => ({ threw: result.threw, ast: result.ast && { errors: result.ast.errors }, diff --git a/packages/babel-parser/test/helpers/serialization.js b/packages/babel-parser/test/helpers/serialization.js index 006c4ffa1f36..5b142ad0c106 100644 --- a/packages/babel-parser/test/helpers/serialization.js +++ b/packages/babel-parser/test/helpers/serialization.js @@ -21,17 +21,53 @@ const toBigInt = global.BigInt || (() => null); const SerializationKey = "$$ babel internal serialized type"; /* eslint-disable no-confusing-arrow */ -export const deserialize = (filename, string) => - JSONParse( - string, - isSerialized(filename) && - ((key, value) => - key !== "value" || - !value || - typeof value !== "object" || - !value[SerializationKey] - ? value - : value[SerializationKey] === "RegExp" - ? new RegExp(value.source, value.flags) - : toBigInt(value.value)), +export const deserialize = (filename, options, string) => + withErrors( + options.throws, + !!string && + JSONParse( + string, + isSerialized(filename) && + ((key, value) => + key !== "value" || + !value || + typeof value !== "object" || + !value[SerializationKey] + ? value + : value[SerializationKey] === "RegExp" + ? new RegExp(value.source, value.flags) + : toBigInt(value.value)), + ), ); + +// For now we assemble this structure here, but in the future we should just +// store the entire thing into output.json, instead of thrown errors in +// options.json. If we end up going with +// https://github.com/babel/babel/issues/14175, then as a side-effect we'll +// actually get this behavior for free, since everything will always just be +// store in the errors array. +function withErrors(throws, ast) { + const threw = !!throws && toError(throws); + const errors = !!ast && !!ast.errors && ast.errors.map(toError); + + return { threw, ast: errors ? { ...ast, errors } : ast }; +} + +// This is more complicated than it needs to be because for unfortunately thrown +// errors and recovered errors are serialized slightly differently. Thrown +// errors are serialized *without* their corresponding class name (the result of +// calling .toString() on the error), while errors in the errors array of the +// AST are serialized *with* the class name (the result of just storing the +// contents of the message). As such, the type information is lost for thrown +// errors, but it just happens to be that they're all SyntaxErrors. +// +// Because of this, we have to account for both cases here, so if the name is +// present, we use it, otherwise, we just assume it's a SyntaxError. In the +// future, we should serialize the errors into an object representation instead +// of just a string, which we can use to additionally store the location info so +// we can test that too. +const ErrorPrefixRegExp = /^[A-Za-z]*Error:\s/; +const toError = message => + /^Error/.test(message.replace(ErrorPrefixRegExp, "")) + ? Error(message.replace(ErrorPrefixRegExp, "")) + : SyntaxError(message.replace(ErrorPrefixRegExp, "")); diff --git a/packages/babel-parser/test/helpers/to-fuzzed-options.js b/packages/babel-parser/test/helpers/to-fuzzed-options.js index 30170e690a33..4bcfaf3a9f70 100644 --- a/packages/babel-parser/test/helpers/to-fuzzed-options.js +++ b/packages/babel-parser/test/helpers/to-fuzzed-options.js @@ -45,18 +45,12 @@ const toAdjustedSyntaxError = (adjust, error) => ) : error; -const DefaultAdjust = toAdjust({ - threw: (adjust, error) => error && toAdjustedSyntaxError(adjust, error), - errors: (adjust, errors) => - errors && errors.map(error => toAdjustedSyntaxError(adjust, error)), -}); - export default function toFuzzedOptions(options) { // Don't fuzz test in node 8, since for whatever reason Jest 23 on node 8 // takes forever after a certain number of tests, even if those tests are // empty. if (/v8\./.test(process.version)) { - return [[DefaultAdjust, options]]; + return [[false, options]]; } const { startLine = 1, startColumn = 0 } = options; @@ -98,7 +92,6 @@ export default function toFuzzedOptions(options) { .map(options => [options, options.startLine || 1, options.startColumn || 0]) .map(([options, fStartLine, fStartColumn]) => [ toAdjust({ - ...DefaultAdjust, ...(startLine !== fStartLine && { line: (_, line) => line - startLine + fStartLine, }), @@ -108,5 +101,16 @@ export default function toFuzzedOptions(options) { }), }), options, + ]) + .map(([adjust, options]) => [ + adjust && + toAdjust({ + ...adjust, + threw: (adjust, error) => + error && toAdjustedSyntaxError(adjust, error), + errors: (adjust, errors) => + errors && errors.map(error => toAdjustedSyntaxError(adjust, error)), + }), + options, ]); } From e9fc21121e6c74b5e1f592fa0fe70a0bbb3cb188 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Sun, 23 Jan 2022 12:46:59 -0800 Subject: [PATCH 09/34] Fix only storing cause if it's an error. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/fixture-error.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index 2e8ce61ab939..9844709a0f92 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -58,9 +58,8 @@ Object.assign( super(toMessage(difference, ...args), difference); this.cause = - (!!(difference.actual instanceof Error) && - difference.actual.context) || - difference.actual; + (difference.actual instanceof Error) && + (difference.actual.context || difference.actual); } // Don't show the stack of FixtureErrors, it's irrelevant. From 00fb3f6ecd38d8049fae1499ae78b2e23e4d9a0d Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Sun, 23 Jan 2022 13:40:46 -0800 Subject: [PATCH 10/34] Fix UnexpectedSuccess error. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/fixture-error.js | 6 +++--- packages/babel-parser/test/helpers/run-fixture-tests.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index 9844709a0f92..6f6beb69860d 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -19,7 +19,7 @@ export default class FixtureError extends Error { ? new FixtureError.UnexpectedError(difference) : difference.actual ? new FixtureError.DifferentError(difference) - : actual.ast.errors + : actual.ast && actual.ast.errors ? new FixtureError.UnexpectedRecovery(difference, actual.ast.errors) : new FixtureError.UnexpectedSuccess(difference); } @@ -38,11 +38,11 @@ Object.assign( UnexpectedError: () => `Encountered unexpected unrecoverable error.\n`, UnexpectedSuccess: ({ expected }) => - `Expected non-recoverable error message:\n\n${expected}\n\n` + + `Expected unrecoverable error:\n\n ${expected}\n\n` + `But parsing succeeded without errors.`, UnexpectedRecovery: ({ expected }, errors) => - `Expected non-recoverable error message:\n\n${expected}\n\n` + + `Expected unrecoverable error:\n\n ${expected}\n\n` + `But instead parsing recovered from errors:\n\n${JSON.stringify( errors, null, diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 74fb9b8ed608..94de60a7683a 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -95,7 +95,7 @@ function runParseTest(parse, test, onlyCompareErrors) { ); if (difference !== Difference.None) { - throw FixtureError.fromDifference(difference); + throw FixtureError.fromDifference(difference, actual); } } From 63a3e0c606627c10af47ed969c0f18d9b2440a1d Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Sun, 23 Jan 2022 14:12:01 -0800 Subject: [PATCH 11/34] Better DifferentError and cleaned up the cause stuff a bit. Reviewed by @tolmasky. --- .../babel-parser/test/helpers/fixture-error.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index 6f6beb69860d..7b220999de82 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -16,9 +16,9 @@ export default class FixtureError extends Error { return difference.path[0] !== "threw" ? new FixtureError.DifferentAST(difference) : !difference.expected - ? new FixtureError.UnexpectedError(difference) + ? new FixtureError.UnexpectedError(difference, actual.threw) : difference.actual - ? new FixtureError.DifferentError(difference) + ? new FixtureError.DifferentError(difference, actual.threw) : actual.ast && actual.ast.errors ? new FixtureError.UnexpectedRecovery(difference, actual.ast.errors) : new FixtureError.UnexpectedSuccess(difference); @@ -29,9 +29,9 @@ Object.assign( FixtureError, mapEntries( { - DifferentError: ({ expected, actual }) => - `Expected error message:\n\n${expected}\n\n` + - `But instead found error message:\n\n${actual}`, + DifferentError: ({ expected }) => + `Expected unrecoverable error: \n\n${expected}\n\n`+ + `But instead encountered different unrecoverable error.\n`, DifferentAST: ({ message }) => message, @@ -54,12 +54,10 @@ Object.assign( named( name, class extends FixtureError { - constructor(difference, ...args) { - super(toMessage(difference, ...args), difference); + constructor(difference, cause) { + super(toMessage(difference, cause), difference); - this.cause = - (difference.actual instanceof Error) && - (difference.actual.context || difference.actual); + this.cause = (cause instanceof Error) && (cause.context || cause); } // Don't show the stack of FixtureErrors, it's irrelevant. From e5933b332a8aa4bf4bbdedf3071defd387c1f76e Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Sun, 23 Jan 2022 16:15:58 -0800 Subject: [PATCH 12/34] Fix linter errors. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/fixture-error.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index 7b220999de82..bb1983798689 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -30,7 +30,7 @@ Object.assign( mapEntries( { DifferentError: ({ expected }) => - `Expected unrecoverable error: \n\n${expected}\n\n`+ + `Expected unrecoverable error: \n\n${expected}\n\n` + `But instead encountered different unrecoverable error.\n`, DifferentAST: ({ message }) => message, @@ -57,7 +57,7 @@ Object.assign( constructor(difference, cause) { super(toMessage(difference, cause), difference); - this.cause = (cause instanceof Error) && (cause.context || cause); + if (cause) this.cause = cause.context || cause; } // Don't show the stack of FixtureErrors, it's irrelevant. From 3a555d479b85fc37ab82f3026ffed477a0ca2e23 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 06:17:35 -0800 Subject: [PATCH 13/34] First pass at serialization. Reviewed by @tolmasky. --- .../test/helpers/serialization.js | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/babel-parser/test/helpers/serialization.js b/packages/babel-parser/test/helpers/serialization.js index 5b142ad0c106..02a1de77d396 100644 --- a/packages/babel-parser/test/helpers/serialization.js +++ b/packages/babel-parser/test/helpers/serialization.js @@ -1,4 +1,4 @@ -const { parse: JSONParse } = JSON; +const { parse: JSONParse, stringify } = JSON; // We give JSON files that needed our special serialization the extension // ".serialized.json" instead of just ".json" so that we can only use our @@ -9,7 +9,7 @@ const isSerialized = filename => /\.serialized\.json$/.test(filename); // We've only serialized one BigInt in the entire test suite: // -// packages/babel-parser/test/fixtures/estree/basic/bigint/output.serialized.json +// packages/babel-parser/test/fixtures/estree/bigInt/basic/output.serialized.json // // This is because only estree actually includes the BigInt value in the Literal // node. If the JS environemnt doesn't support bigint, then estree will just @@ -71,3 +71,26 @@ const toError = message => /^Error/.test(message.replace(ErrorPrefixRegExp, "")) ? Error(message.replace(ErrorPrefixRegExp, "")) : SyntaxError(message.replace(ErrorPrefixRegExp, "")); + +const LocRegExp = /"loc":(\s*\{(?:[^}{]+|\{(?:[^}{]+|\([^}{]*\})*\})*\})/gm; + +export function serialize(value) { + let encoded = false; + const toEncoded = (name, data) => ( + (encoded = true), { [SerializationKey]: name, ...data } + ); + const encode = (_, value) => + typeof value === "bigint" + ? toEncoded("BigInt", { value: value + "" }) + : value instanceof RegExp + ? toEncoded("RegExp", { source: value.source, flags: value.flags }) + : value; + const serialized = stringify(value, encode, 2).replace( + LocRegExp, + // This is safe since none of the values can have spaces in them, otherwise + // we could also do `stringify(parse(string), null, 0)`. + (_, string) => `"loc":${string.replace(/\s+/gm, () => "")}`, + ); + + return [encoded, serialized]; +} From 5648fc024f097a6b466bc5c86fcba9474462b99e Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 08:05:39 -0800 Subject: [PATCH 14/34] Better errors and serialization. Reviewed by @tolmasky. --- .../test/helpers/fixture-error.js | 19 ++++++++++--------- .../test/helpers/serialization.js | 4 +++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index bb1983798689..e4f11c7a3616 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -1,6 +1,7 @@ import { inspect } from "util"; import "./polyfill.js"; +const { isArray } = Array; const { defineProperty, entries, fromEntries } = Object; const named = (name, object) => defineProperty(object, "name", { value: name }); @@ -31,11 +32,11 @@ Object.assign( { DifferentError: ({ expected }) => `Expected unrecoverable error: \n\n${expected}\n\n` + - `But instead encountered different unrecoverable error.\n`, + `But instead encountered different unrecoverable error.`, DifferentAST: ({ message }) => message, - UnexpectedError: () => `Encountered unexpected unrecoverable error.\n`, + UnexpectedError: () => `Encountered unexpected unrecoverable error.`, UnexpectedSuccess: ({ expected }) => `Expected unrecoverable error:\n\n ${expected}\n\n` + @@ -43,11 +44,7 @@ Object.assign( UnexpectedRecovery: ({ expected }, errors) => `Expected unrecoverable error:\n\n ${expected}\n\n` + - `But instead parsing recovered from errors:\n\n${JSON.stringify( - errors, - null, - 2, - )}`, + `But instead parsing recovered from ${errors.length} errors.`, }, ([name, toMessage]) => [ name, @@ -57,13 +54,17 @@ Object.assign( constructor(difference, cause) { super(toMessage(difference, cause), difference); - if (cause) this.cause = cause.context || cause; + if (cause) { + this.cause = isArray(cause) + ? cause.map(cause => cause.context || cause) + : cause; + } } // Don't show the stack of FixtureErrors, it's irrelevant. // Instead, show the cause, if present. [inspect.custom](depth, options) { - return `${this.message.replace(/\.\n$/, ":\n")}${ + return `${this.message.replace(/(?<=error(s?))\.$/, ":\n")}${ this.cause ? `\n${inspect(this.cause, options)}`.replace(/\n/g, "\n ") : "" diff --git a/packages/babel-parser/test/helpers/serialization.js b/packages/babel-parser/test/helpers/serialization.js index 02a1de77d396..06d6a452a5b5 100644 --- a/packages/babel-parser/test/helpers/serialization.js +++ b/packages/babel-parser/test/helpers/serialization.js @@ -79,11 +79,13 @@ export function serialize(value) { const toEncoded = (name, data) => ( (encoded = true), { [SerializationKey]: name, ...data } ); - const encode = (_, value) => + const encode = (key, value) => typeof value === "bigint" ? toEncoded("BigInt", { value: value + "" }) : value instanceof RegExp ? toEncoded("RegExp", { source: value.source, flags: value.flags }) + : value instanceof Error + ? value + "" : value; const serialized = stringify(value, encode, 2).replace( LocRegExp, From dc186b1815c07d49db1e504934ad12ec14c9b792 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 09:43:39 -0800 Subject: [PATCH 15/34] Fix saving output. Reviewed by @tolmasky. --- ...t.serialized.json => output.extended.json} | 2 +- .../test/helpers/fixture-error.js | 9 ++- .../test/helpers/run-fixture-tests.js | 59 ++++++++++++------- .../test/helpers/serialization.js | 16 ++--- 4 files changed, 54 insertions(+), 32 deletions(-) rename packages/babel-parser/test/fixtures/estree/bigInt/basic/{output.serialized.json => output.extended.json} (94%) diff --git a/packages/babel-parser/test/fixtures/estree/bigInt/basic/output.serialized.json b/packages/babel-parser/test/fixtures/estree/bigInt/basic/output.extended.json similarity index 94% rename from packages/babel-parser/test/fixtures/estree/bigInt/basic/output.serialized.json rename to packages/babel-parser/test/fixtures/estree/bigInt/basic/output.extended.json index 610eff55feff..5442b033cf63 100644 --- a/packages/babel-parser/test/fixtures/estree/bigInt/basic/output.serialized.json +++ b/packages/babel-parser/test/fixtures/estree/bigInt/basic/output.extended.json @@ -23,7 +23,7 @@ "type": "Literal", "start":10,"end":12,"loc":{"start":{"line":1,"column":10},"end":{"line":1,"column":12}}, "value": { - "$$ babel internal serialized type": "BigInt", + "$$ babel internal serialized type": "bigint", "value": "1" }, "raw": "1n", diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index e4f11c7a3616..1be595410b80 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -1,4 +1,5 @@ import { inspect } from "util"; +import Difference from "./difference.js"; import "./polyfill.js"; const { isArray } = Array; @@ -14,7 +15,9 @@ export default class FixtureError extends Error { } static fromDifference(difference, actual) { - return difference.path[0] !== "threw" + return difference === Difference.None + ? FixtureError.None + : difference.path[0] !== "threw" ? new FixtureError.DifferentAST(difference) : !difference.expected ? new FixtureError.UnexpectedError(difference, actual.threw) @@ -26,6 +29,10 @@ export default class FixtureError extends Error { } } +FixtureError.None = Object.freeze( + Object.setPrototypeOf({}, FixtureError.prototype), +); + Object.assign( FixtureError, mapEntries( diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 94de60a7683a..37b876e0253c 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -1,13 +1,13 @@ import { multiple as getFixtures } from "@babel/helper-fixtures"; -import fs from "fs"; -import { dirname } from "path"; +import { writeFileSync, existsSync, unlinkSync } from "fs"; +import { join } from "path"; import Difference from "./difference.js"; import FixtureError from "./fixture-error.js"; import toFuzzedOptions from "./to-fuzzed-options.js"; -import { deserialize } from "./serialization.js"; +import { serialize, deserialize } from "./serialization.js"; import toContextualSyntaxError from "./to-contextual-syntax-error.js"; -const { stringify } = JSON; +const { OVERWRITE } = process.env; export default function runFixtureTests( fixturesPath, @@ -45,7 +45,7 @@ function runAutogeneratedParseTests( `(${startLine}, ${startColumn})`; toFuzzedOptions(options) - .map(([adjust, options]) => ({ + .map(([adjust, options], index) => ({ ...task, title: `${title} start = ${toStartPosition(options)}`, adjust, @@ -53,23 +53,10 @@ function runAutogeneratedParseTests( expected, filename: task.actual.loc, source: task.actual.code, + original: index === 0, })) .forEach(test => - testFn(test.title, function () { - try { - runParseTest(parse, test, onlyCompareErrors); - } catch (err) { - if (!test.expect.code && !process.env.CI) { - const fn = dirname(task.expect.loc) + "/options.json"; - if (!fs.existsSync(fn)) { - test.options = task.options || {}; - test.options.throws = err.originalMessage || err.message; - fs.writeFileSync(fn, stringify(task.options, null, 2)); - } - } - throw err; - } - }), + testFn(test.title, () => runParseTest(parse, test, onlyCompareErrors)), ); } @@ -93,10 +80,38 @@ function runParseTest(parse, test, onlyCompareErrors) { onlyCompareErrors ? toJustErrors(expected) : expected, onlyCompareErrors ? toJustErrors(actual) : actual, ); + const error = FixtureError.fromDifference(difference, actual); - if (difference !== Difference.None) { - throw FixtureError.fromDifference(difference, actual); + // No differences means we passed and there's nothing left to do. + if (error === FixtureError.None) return; + + // If we're not overwriting the current values with whatever we get this time + // around, then we have a legitimate error that we need to report. + if (!OVERWRITE) throw error; + + // We only write the output of the original test, not all it's auto-generated + // variations. + if (!test.original) return; + + // When only comparing errors, we don't want to overwrite the AST JSON because + // it belongs to a different test. + if (onlyCompareErrors) return; + + const [extended, serialized] = serialize(actual.ast); + const testLocation = test.optionsDir; + const extension = extended ? ".extended.json" : ".json"; + const outputLocation = join(testLocation, `output${extension}`); + + const reverseExtension = extended ? ".json" : ".extended.json"; + const previousLocation = join(testLocation, `output${reverseExtension}`); + + // If we're switching formats, make sure to remove the old format. + if (previousLocation !== outputLocation && existsSync(previousLocation)) { + unlinkSync(reverseExtension); } + + // Remove the other one... + writeFileSync(outputLocation, serialized, "utf-8"); } function parseWithRecovery(parse, source, filename, options) { diff --git a/packages/babel-parser/test/helpers/serialization.js b/packages/babel-parser/test/helpers/serialization.js index 06d6a452a5b5..b8561156faf9 100644 --- a/packages/babel-parser/test/helpers/serialization.js +++ b/packages/babel-parser/test/helpers/serialization.js @@ -5,7 +5,7 @@ const { parse: JSONParse, stringify } = JSON; // deserialization function in those cases (which is slower), and in all // other instances just rely on normal JSON.parse with no deserialization // function. -const isSerialized = filename => /\.serialized\.json$/.test(filename); +const isExtended = filename => /\.extended\.json$/.test(filename); // We've only serialized one BigInt in the entire test suite: // @@ -27,7 +27,7 @@ export const deserialize = (filename, options, string) => !!string && JSONParse( string, - isSerialized(filename) && + isExtended(filename) && ((key, value) => key !== "value" || !value || @@ -75,15 +75,15 @@ const toError = message => const LocRegExp = /"loc":(\s*\{(?:[^}{]+|\{(?:[^}{]+|\([^}{]*\})*\})*\})/gm; export function serialize(value) { - let encoded = false; - const toEncoded = (name, data) => ( - (encoded = true), { [SerializationKey]: name, ...data } + let extended = false; + const toExended = (name, data) => ( + (extended = true), { [SerializationKey]: name, ...data } ); const encode = (key, value) => typeof value === "bigint" - ? toEncoded("BigInt", { value: value + "" }) + ? toExended("bigint", { value: value + "" }) : value instanceof RegExp - ? toEncoded("RegExp", { source: value.source, flags: value.flags }) + ? toExended("RegExp", { source: value.source, flags: value.flags }) : value instanceof Error ? value + "" : value; @@ -94,5 +94,5 @@ export function serialize(value) { (_, string) => `"loc":${string.replace(/\s+/gm, () => "")}`, ); - return [encoded, serialized]; + return [extended, serialized]; } From c31d5bad703b1b106ceeb455c0eafa81406a4f01 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 10:55:37 -0800 Subject: [PATCH 16/34] Fix saving options. Reviewed by @tolmasky. --- packages/babel-helper-fixtures/src/index.ts | 3 ++- ...t.serialized.json => output.extended.json} | 0 .../test/helpers/run-fixture-tests.js | 23 ++++++++++++++++++- .../test/helpers/serialization.js | 4 ++-- 4 files changed, 26 insertions(+), 4 deletions(-) rename packages/babel-parser/test/fixtures/estree/literal/regexp/{output.serialized.json => output.extended.json} (100%) diff --git a/packages/babel-helper-fixtures/src/index.ts b/packages/babel-helper-fixtures/src/index.ts index ee3a463ba79b..ade006ccb250 100644 --- a/packages/babel-helper-fixtures/src/index.ts +++ b/packages/babel-helper-fixtures/src/index.ts @@ -105,7 +105,7 @@ function pushTask(taskName, taskDir, suite, suiteName) { const expectLoc = findFile(taskDir + "/output", true /* allowJSON */) || - findFile(`${taskDir}/output.serialized`, true) || + findFile(`${taskDir}/output.extended`, true) || taskDir + "/output.js"; const stdoutLoc = taskDir + "/stdout.txt"; const stderrLoc = taskDir + "/stderr.txt"; @@ -131,6 +131,7 @@ function pushTask(taskName, taskDir, suite, suiteName) { if (taskOptsLoc) Object.assign(taskOpts, require(taskOptsLoc)); const test = { + taskDir, optionsDir: taskOptsLoc ? path.dirname(taskOptsLoc) : null, title: humanize(taskName, true), disabled: diff --git a/packages/babel-parser/test/fixtures/estree/literal/regexp/output.serialized.json b/packages/babel-parser/test/fixtures/estree/literal/regexp/output.extended.json similarity index 100% rename from packages/babel-parser/test/fixtures/estree/literal/regexp/output.serialized.json rename to packages/babel-parser/test/fixtures/estree/literal/regexp/output.extended.json diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 37b876e0253c..6c73b48e26ce 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -8,6 +8,7 @@ import { serialize, deserialize } from "./serialization.js"; import toContextualSyntaxError from "./to-contextual-syntax-error.js"; const { OVERWRITE } = process.env; +const { stringify } = JSON; export default function runFixtureTests( fixturesPath, @@ -54,6 +55,7 @@ function runAutogeneratedParseTests( filename: task.actual.loc, source: task.actual.code, original: index === 0, + originalOptions: task.options, })) .forEach(test => testFn(test.title, () => runParseTest(parse, test, onlyCompareErrors)), @@ -93,12 +95,31 @@ function runParseTest(parse, test, onlyCompareErrors) { // variations. if (!test.original) return; + const testLocation = test.taskDir; + + // FIXME: We're just maintaining the legacy behavior of storing *just* the + // error `message` here, which differs from the error's `toString()` that we + // store for each error in the `errors` array. In both cases, we should + // serialize the full error to be able to property test locations, + // reasonCodes, etc. + const throws = !!actual.threw && actual.threw.message; + const { throws: _, ...oldOptions } = test.originalOptions; + const newOptions = { ...oldOptions, ...(throws && { throws }) }; + const optionsLocation = join(testLocation, "options.json"); + + // Store (or overwrite) the options file if there's anything to record, + // otherwise remove it. + if (Object.keys(newOptions).length > 0) { + writeFileSync(optionsLocation, stringify(newOptions, null, 2), "utf-8"); + } else if (existsSync(optionsLocation)) { + unlinkSync(optionsLocation); + } + // When only comparing errors, we don't want to overwrite the AST JSON because // it belongs to a different test. if (onlyCompareErrors) return; const [extended, serialized] = serialize(actual.ast); - const testLocation = test.optionsDir; const extension = extended ? ".extended.json" : ".json"; const outputLocation = join(testLocation, `output${extension}`); diff --git a/packages/babel-parser/test/helpers/serialization.js b/packages/babel-parser/test/helpers/serialization.js index b8561156faf9..c037dc52e8bd 100644 --- a/packages/babel-parser/test/helpers/serialization.js +++ b/packages/babel-parser/test/helpers/serialization.js @@ -1,7 +1,7 @@ const { parse: JSONParse, stringify } = JSON; // We give JSON files that needed our special serialization the extension -// ".serialized.json" instead of just ".json" so that we can only use our +// ".extended.json" instead of just ".json" so that we can only use our // deserialization function in those cases (which is slower), and in all // other instances just rely on normal JSON.parse with no deserialization // function. @@ -9,7 +9,7 @@ const isExtended = filename => /\.extended\.json$/.test(filename); // We've only serialized one BigInt in the entire test suite: // -// packages/babel-parser/test/fixtures/estree/bigInt/basic/output.serialized.json +// packages/babel-parser/test/fixtures/estree/bigInt/basic/output.extended.json // // This is because only estree actually includes the BigInt value in the Literal // node. If the JS environemnt doesn't support bigint, then estree will just From a275b7f5ca26f124e2e3df582f6cfed52cf235a7 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 11:13:16 -0800 Subject: [PATCH 17/34] Fix linter errors and incorrect removal. Reviewed by @tolmasky. --- .../test/helpers/run-fixture-tests.js | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 6c73b48e26ce..3c6f92eee6cd 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -103,7 +103,11 @@ function runParseTest(parse, test, onlyCompareErrors) { // serialize the full error to be able to property test locations, // reasonCodes, etc. const throws = !!actual.threw && actual.threw.message; + + // We want to throw away the contents of `throw` here. + // eslint-disable-next-line no-unused-vars const { throws: _, ...oldOptions } = test.originalOptions; + const newOptions = { ...oldOptions, ...(throws && { throws }) }; const optionsLocation = join(testLocation, "options.json"); @@ -119,20 +123,24 @@ function runParseTest(parse, test, onlyCompareErrors) { // it belongs to a different test. if (onlyCompareErrors) return; - const [extended, serialized] = serialize(actual.ast); - const extension = extended ? ".extended.json" : ".json"; - const outputLocation = join(testLocation, `output${extension}`); + const normalLocation = join(testLocation, "output.json"); + const extendedLocation = join(testLocation, "output.extended.json"); - const reverseExtension = extended ? ".json" : ".extended.json"; - const previousLocation = join(testLocation, `output${reverseExtension}`); + const [extended, serialized] = actual.ast ? serialize(actual.ast) : []; + const outputLocation = + serialized && (extended ? extendedLocation : normalLocation); - // If we're switching formats, make sure to remove the old format. - if (previousLocation !== outputLocation && existsSync(previousLocation)) { - unlinkSync(reverseExtension); + if (outputLocation) { + writeFileSync(outputLocation, serialized, "utf-8"); } - // Remove the other one... - writeFileSync(outputLocation, serialized, "utf-8"); + // Remove any previous output files that are no longer valid, either because + // extension changed, or because we aren't writing it out at all anymore. + for (location of [normalLocation, extendedLocation]) { + if (location !== outputLocation && existsSync(location)) { + unlinkSync(location); + } + } } function parseWithRecovery(parse, source, filename, options) { From c6b28ece045eef91ba2643ea91614202525483f1 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 11:13:48 -0800 Subject: [PATCH 18/34] Add FUZZ environment variable. Reviewed by @tolmasky. --- .github/workflows/ci.yml | 9 +++++++++ packages/babel-parser/test/helpers/to-fuzzed-options.js | 9 ++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a86304513628..c7520a36742b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -183,6 +183,15 @@ jobs: with: node-version: ${{ matrix.node-version }} - name: Test on node.js ${{ matrix.node-version }} + if: matrix.node-version == '8' + # Hack: --color has supports-color@5 returned true for GitHub CI + # Remove once `chalk` is bumped to 4.0. + + # Todo(Babel 8): Jest execution path is hardcoded because Yarn 2 does not support node 6 + run: | + BABEL_ENV=test FUZZ=false node --max-old-space-size=4096 ./node_modules/.bin/jest --ci --color + - name: Test on node.js ${{ matrix.node-version }} + if: matrix.node-version != '8' # Hack: --color has supports-color@5 returned true for GitHub CI # Remove once `chalk` is bumped to 4.0. diff --git a/packages/babel-parser/test/helpers/to-fuzzed-options.js b/packages/babel-parser/test/helpers/to-fuzzed-options.js index 4bcfaf3a9f70..a74b25f11296 100644 --- a/packages/babel-parser/test/helpers/to-fuzzed-options.js +++ b/packages/babel-parser/test/helpers/to-fuzzed-options.js @@ -2,7 +2,7 @@ const random = (min, max) => Math.floor(Math.random() * (max - min + 1) + min); const clone = value => JSON.parse(JSON.stringify(value)); -// const dedupe = array => Object.values(Object.fromEntries(array)); +const { FUZZ } = process.env; const toDescriptorAssignedObject = (delta, object) => delta.reduce( @@ -46,12 +46,7 @@ const toAdjustedSyntaxError = (adjust, error) => : error; export default function toFuzzedOptions(options) { - // Don't fuzz test in node 8, since for whatever reason Jest 23 on node 8 - // takes forever after a certain number of tests, even if those tests are - // empty. - if (/v8\./.test(process.version)) { - return [[false, options]]; - } + if (FUZZ === "false") return [[false, options]]; const { startLine = 1, startColumn = 0 } = options; From a040ebde5f05e04902c7a5f7b313fbf2df3deadf Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 11:47:00 -0800 Subject: [PATCH 19/34] Fix location undefined problem when saving. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/run-fixture-tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 3c6f92eee6cd..5813a30035ed 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -136,7 +136,7 @@ function runParseTest(parse, test, onlyCompareErrors) { // Remove any previous output files that are no longer valid, either because // extension changed, or because we aren't writing it out at all anymore. - for (location of [normalLocation, extendedLocation]) { + for (const location of [normalLocation, extendedLocation]) { if (location !== outputLocation && existsSync(location)) { unlinkSync(location); } From 0455aeb5ed1b8a67748df351edff20530892b2c6 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 12:36:58 -0800 Subject: [PATCH 20/34] Read the actual options file in since we don't have a true copy of the original options. Reviewed by @tolmasky. --- .../test/helpers/run-fixture-tests.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 5813a30035ed..af6502102c74 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -1,5 +1,5 @@ import { multiple as getFixtures } from "@babel/helper-fixtures"; -import { writeFileSync, existsSync, unlinkSync } from "fs"; +import { existsSync, readFileSync, unlinkSync, writeFileSync } from "fs"; import { join } from "path"; import Difference from "./difference.js"; import FixtureError from "./fixture-error.js"; @@ -8,7 +8,7 @@ import { serialize, deserialize } from "./serialization.js"; import toContextualSyntaxError from "./to-contextual-syntax-error.js"; const { OVERWRITE } = process.env; -const { stringify } = JSON; +const { stringify, parse: JSONParse } = JSON; export default function runFixtureTests( fixturesPath, @@ -55,7 +55,6 @@ function runAutogeneratedParseTests( filename: task.actual.loc, source: task.actual.code, original: index === 0, - originalOptions: task.options, })) .forEach(test => testFn(test.title, () => runParseTest(parse, test, onlyCompareErrors)), @@ -103,13 +102,12 @@ function runParseTest(parse, test, onlyCompareErrors) { // serialize the full error to be able to property test locations, // reasonCodes, etc. const throws = !!actual.threw && actual.threw.message; + const optionsLocation = join(testLocation, "options.json"); - // We want to throw away the contents of `throw` here. + // We want to throw away the contents of `throws` here. // eslint-disable-next-line no-unused-vars - const { throws: _, ...oldOptions } = test.originalOptions; - + const { throws: _, ...oldOptions } = readJSON(optionsLocation); const newOptions = { ...oldOptions, ...(throws && { throws }) }; - const optionsLocation = join(testLocation, "options.json"); // Store (or overwrite) the options file if there's anything to record, // otherwise remove it. @@ -143,6 +141,12 @@ function runParseTest(parse, test, onlyCompareErrors) { } } +function readJSON(filename) { + try { + return JSONParse(readFileSync(filename, "utf-8")); + } catch (error) { return {}; } +} + function parseWithRecovery(parse, source, filename, options) { try { const ast = parse(source, { errorRecovery: true, ...options }); From c7034182ec0714ef8f1cbf857107048b0f88922c Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 13:07:12 -0800 Subject: [PATCH 21/34] Fix also compacting start and end outside of loc. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/serialization.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/babel-parser/test/helpers/serialization.js b/packages/babel-parser/test/helpers/serialization.js index c037dc52e8bd..e925e402ad68 100644 --- a/packages/babel-parser/test/helpers/serialization.js +++ b/packages/babel-parser/test/helpers/serialization.js @@ -73,6 +73,8 @@ const toError = message => : SyntaxError(message.replace(ErrorPrefixRegExp, "")); const LocRegExp = /"loc":(\s*\{(?:[^}{]+|\{(?:[^}{]+|\([^}{]*\})*\})*\})/gm; +const StartEndRegExp = /("(start|end)":\s*(\d+),\s*){2}/gm; +const CompactRegExp = new RegExp(`${StartEndRegExp.source}${LocRegExp.source}`, "gm"); export function serialize(value) { let extended = false; @@ -88,10 +90,9 @@ export function serialize(value) { ? value + "" : value; const serialized = stringify(value, encode, 2).replace( - LocRegExp, - // This is safe since none of the values can have spaces in them, otherwise - // we could also do `stringify(parse(string), null, 0)`. - (_, string) => `"loc":${string.replace(/\s+/gm, () => "")}`, + CompactRegExp, + // This is safe since none of the values can have spaces in them. + string => string.replace(/\s+/gm, () => ""), ); return [extended, serialized]; From a64178aa53696015ddc7dae6ac83effa3af612a3 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 13:21:25 -0800 Subject: [PATCH 22/34] Fix linter error. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/run-fixture-tests.js | 4 +++- packages/babel-parser/test/helpers/serialization.js | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index af6502102c74..767be5007edc 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -144,7 +144,9 @@ function runParseTest(parse, test, onlyCompareErrors) { function readJSON(filename) { try { return JSONParse(readFileSync(filename, "utf-8")); - } catch (error) { return {}; } + } catch (error) { + return {}; + } } function parseWithRecovery(parse, source, filename, options) { diff --git a/packages/babel-parser/test/helpers/serialization.js b/packages/babel-parser/test/helpers/serialization.js index e925e402ad68..d2bc819e4688 100644 --- a/packages/babel-parser/test/helpers/serialization.js +++ b/packages/babel-parser/test/helpers/serialization.js @@ -74,7 +74,10 @@ const toError = message => const LocRegExp = /"loc":(\s*\{(?:[^}{]+|\{(?:[^}{]+|\([^}{]*\})*\})*\})/gm; const StartEndRegExp = /("(start|end)":\s*(\d+),\s*){2}/gm; -const CompactRegExp = new RegExp(`${StartEndRegExp.source}${LocRegExp.source}`, "gm"); +const CompactRegExp = new RegExp( + `${StartEndRegExp.source}${LocRegExp.source}`, + "gm", +); export function serialize(value) { let extended = false; From ffb642e15e3163f14937783ce19a083e58fc74e9 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 16:03:55 -0800 Subject: [PATCH 23/34] Address a few style issues. Reviewed by @tolmasky. --- .github/workflows/ci.yml | 2 +- packages/babel-parser/test/helpers/difference.js | 5 ++--- packages/babel-parser/test/helpers/polyfill.js | 1 + packages/babel-parser/test/helpers/to-fuzzed-options.js | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7520a36742b..9f6c3c12cbeb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -189,7 +189,7 @@ jobs: # Todo(Babel 8): Jest execution path is hardcoded because Yarn 2 does not support node 6 run: | - BABEL_ENV=test FUZZ=false node --max-old-space-size=4096 ./node_modules/.bin/jest --ci --color + BABEL_ENV=test TEST_FUZZ=false node --max-old-space-size=4096 ./node_modules/.bin/jest --ci --color - name: Test on node.js ${{ matrix.node-version }} if: matrix.node-version != '8' # Hack: --color has supports-color@5 returned true for GitHub CI diff --git a/packages/babel-parser/test/helpers/difference.js b/packages/babel-parser/test/helpers/difference.js index 0a279eebcc91..c6dac717075b 100644 --- a/packages/babel-parser/test/helpers/difference.js +++ b/packages/babel-parser/test/helpers/difference.js @@ -3,6 +3,7 @@ import { isIdentifierName } from "@babel/helper-validator-identifier"; const { isArray } = Array; +const { isInteger } = Number; const { hasOwnProperty } = Object; export default class Difference { @@ -139,11 +140,9 @@ const toExplanationString = ({ discrepancy, expected, actual, key }) => ? `Did not expect a property ${toValueString(key)}` : discrepancy === "missing-key" ? `${toType(actual)} is missing property ${toValueString(key)}` - : discrepancy === "type" - ? `Wrong type "${expected}" != "${actual}"` : `${toValueString(expected)} != ${toValueString(actual)}`; -const isInt = key => parseInt(key, 10) + "" === key; +const isInt = key => isInteger(+key); const toAccess = key => isInt(key) ? `[${key}]` : isIdentifierName(key) ? `.${key}` : `["${key}"]`; diff --git a/packages/babel-parser/test/helpers/polyfill.js b/packages/babel-parser/test/helpers/polyfill.js index 336fbeb22a4e..2f0c2bcbfe7e 100644 --- a/packages/babel-parser/test/helpers/polyfill.js +++ b/packages/babel-parser/test/helpers/polyfill.js @@ -1,3 +1,4 @@ +// TODO(Babel 8): Remove this file. // We run these tests as far back as Node 6, so we need these there. if (!Object.entries) { diff --git a/packages/babel-parser/test/helpers/to-fuzzed-options.js b/packages/babel-parser/test/helpers/to-fuzzed-options.js index a74b25f11296..14f144e9036f 100644 --- a/packages/babel-parser/test/helpers/to-fuzzed-options.js +++ b/packages/babel-parser/test/helpers/to-fuzzed-options.js @@ -1,8 +1,8 @@ /* eslint-disable no-confusing-arrow */ - const random = (min, max) => Math.floor(Math.random() * (max - min + 1) + min); const clone = value => JSON.parse(JSON.stringify(value)); -const { FUZZ } = process.env; + +const { TEST_FUZZ } = process.env; const toDescriptorAssignedObject = (delta, object) => delta.reduce( @@ -46,7 +46,7 @@ const toAdjustedSyntaxError = (adjust, error) => : error; export default function toFuzzedOptions(options) { - if (FUZZ === "false") return [[false, options]]; + if (TEST_FUZZ === "false") return [[false, options]]; const { startLine = 1, startColumn = 0 } = options; From 1a24870b4b7b968cbb71239eeeb7cd19ff401fde Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 16:22:05 -0800 Subject: [PATCH 24/34] Use environment variable for TEST_FUZZ, and disable fuzz testing for Node 6, 8, and 10. Reviewed by @tolmasky. --- .github/workflows/ci.yml | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9f6c3c12cbeb..8098af6a3bbc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -183,21 +183,14 @@ jobs: with: node-version: ${{ matrix.node-version }} - name: Test on node.js ${{ matrix.node-version }} - if: matrix.node-version == '8' - # Hack: --color has supports-color@5 returned true for GitHub CI - # Remove once `chalk` is bumped to 4.0. - - # Todo(Babel 8): Jest execution path is hardcoded because Yarn 2 does not support node 6 - run: | - BABEL_ENV=test TEST_FUZZ=false node --max-old-space-size=4096 ./node_modules/.bin/jest --ci --color - - name: Test on node.js ${{ matrix.node-version }} - if: matrix.node-version != '8' # Hack: --color has supports-color@5 returned true for GitHub CI # Remove once `chalk` is bumped to 4.0. # Todo(Babel 8): Jest execution path is hardcoded because Yarn 2 does not support node 6 run: | BABEL_ENV=test node --max-old-space-size=4096 ./node_modules/.bin/jest --ci --color + env: + TEST_FUZZ: "${{ (matrix.node-version == '6' || matrix.node-version == '8' || matrix.node-version == '10') && 'false' || 'true' }}" test-babel-8-breaking: name: Test Babel 8 breaking changes From 5e760114b5f1650507e12ecec7c605ec74abf80b Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 18:25:55 -0800 Subject: [PATCH 25/34] Address more change requests. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/difference.js | 6 +++--- packages/babel-parser/test/helpers/to-fuzzed-options.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/babel-parser/test/helpers/difference.js b/packages/babel-parser/test/helpers/difference.js index c6dac717075b..23f5e04b6100 100644 --- a/packages/babel-parser/test/helpers/difference.js +++ b/packages/babel-parser/test/helpers/difference.js @@ -129,9 +129,9 @@ const toValueString = (value, type = toType(value)) => ? value.toString() : type === "bigint" ? `${value}n` - : type !== "number" || 1 / value !== -Infinity - ? value + "" - : "-0"; + : Object.is(value, -0) + ? "-0" + : value + "" const toExplanationString = ({ discrepancy, expected, actual, key }) => discrepancy === "length" diff --git a/packages/babel-parser/test/helpers/to-fuzzed-options.js b/packages/babel-parser/test/helpers/to-fuzzed-options.js index 14f144e9036f..bc90e037eb2f 100644 --- a/packages/babel-parser/test/helpers/to-fuzzed-options.js +++ b/packages/babel-parser/test/helpers/to-fuzzed-options.js @@ -15,7 +15,7 @@ const toDescriptorAssignedObject = (delta, object) => clone(object), ); -const toAdjust = adjustments => +const toAdjustFunction = adjustments => !adjustments || Object.keys(adjustments).length === 0 ? null : Object.assign( @@ -86,7 +86,7 @@ export default function toFuzzedOptions(options) { return totalOptions .map(options => [options, options.startLine || 1, options.startColumn || 0]) .map(([options, fStartLine, fStartColumn]) => [ - toAdjust({ + toAdjustFunction({ ...(startLine !== fStartLine && { line: (_, line) => line - startLine + fStartLine, }), @@ -99,7 +99,7 @@ export default function toFuzzedOptions(options) { ]) .map(([adjust, options]) => [ adjust && - toAdjust({ + toAdjustFunction({ ...adjust, threw: (adjust, error) => error && toAdjustedSyntaxError(adjust, error), From 87424e17eaf2a594baf703a7491c5e3c6e72b93b Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 25 Jan 2022 18:51:41 -0800 Subject: [PATCH 26/34] Fix lint error. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/difference.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-parser/test/helpers/difference.js b/packages/babel-parser/test/helpers/difference.js index 23f5e04b6100..60c2567265b8 100644 --- a/packages/babel-parser/test/helpers/difference.js +++ b/packages/babel-parser/test/helpers/difference.js @@ -131,7 +131,7 @@ const toValueString = (value, type = toType(value)) => ? `${value}n` : Object.is(value, -0) ? "-0" - : value + "" + : value + ""; const toExplanationString = ({ discrepancy, expected, actual, key }) => discrepancy === "length" From f6aeb0651250a51ec6b0838a6525a8364e96b82c Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Wed, 26 Jan 2022 07:22:42 -0800 Subject: [PATCH 27/34] Move logic into FixtureError base class and add comments. Reviewed by @tolmasky. --- .../test/helpers/fixture-error.js | 73 +++++++++++-------- .../test/helpers/run-fixture-tests.js | 5 +- .../helpers/to-contextual-syntax-error.js | 10 ++- 3 files changed, 54 insertions(+), 34 deletions(-) diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index 1be595410b80..94a33aeab591 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -7,32 +7,63 @@ const { defineProperty, entries, fromEntries } = Object; const named = (name, object) => defineProperty(object, "name", { value: name }); const mapEntries = (object, f) => fromEntries(entries(object).map(f)); +// eslint-disable-next-line no-confusing-arrow +const toContextError = error => + isArray(error) ? error.map(toContextError) : error.context || error; export default class FixtureError extends Error { - constructor(message, difference) { - super(message); + constructor(difference, { cause } = {}) { + super(); + + // Sigh, still have to manually set the name unfortunately... + named(this.constructor.name, this); + this.difference = difference; + + // Set cause ourselves, since node < 17 has a bug where it won't show it + // otherwise. Technically, we display it ourselves, but best to be defensive + // in case we modify this implementation later. + if (cause) this.cause = cause; + } + + static toMessage() { + return this.constructor.name; + } + + get message() { + return this.constructor.toMessage(this.difference, this.cause); + } + + // Don't show the stack of FixtureErrors, it's irrelevant. + // Instead, show the cause, if present. + [inspect.custom](depth, options) { + return `${this.message.replace(/(?<=error(s?))\.$/, ":\n")}${ + this.cause + ? `\n${inspect(toContextError(this.cause), options)}`.replace( + /\n/g, + "\n ", + ) + : "" + }`; } static fromDifference(difference, actual) { return difference === Difference.None - ? FixtureError.None + ? false : difference.path[0] !== "threw" ? new FixtureError.DifferentAST(difference) : !difference.expected - ? new FixtureError.UnexpectedError(difference, actual.threw) + ? new FixtureError.UnexpectedError(difference, { cause: actual.threw }) : difference.actual - ? new FixtureError.DifferentError(difference, actual.threw) + ? new FixtureError.DifferentError(difference, { cause: actual.threw }) : actual.ast && actual.ast.errors - ? new FixtureError.UnexpectedRecovery(difference, actual.ast.errors) + ? new FixtureError.UnexpectedRecovery(difference, { + cause: actual.ast.errors, + }) : new FixtureError.UnexpectedSuccess(difference); } } -FixtureError.None = Object.freeze( - Object.setPrototypeOf({}, FixtureError.prototype), -); - Object.assign( FixtureError, mapEntries( @@ -56,27 +87,9 @@ Object.assign( ([name, toMessage]) => [ name, named( - name, + `FixtureError.${name}`, class extends FixtureError { - constructor(difference, cause) { - super(toMessage(difference, cause), difference); - - if (cause) { - this.cause = isArray(cause) - ? cause.map(cause => cause.context || cause) - : cause; - } - } - - // Don't show the stack of FixtureErrors, it's irrelevant. - // Instead, show the cause, if present. - [inspect.custom](depth, options) { - return `${this.message.replace(/(?<=error(s?))\.$/, ":\n")}${ - this.cause - ? `\n${inspect(this.cause, options)}`.replace(/\n/g, "\n ") - : "" - }`; - } + static toMessage = toMessage; }, ), ], diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 767be5007edc..2be162ff7d91 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -81,10 +81,11 @@ function runParseTest(parse, test, onlyCompareErrors) { onlyCompareErrors ? toJustErrors(expected) : expected, onlyCompareErrors ? toJustErrors(actual) : actual, ); - const error = FixtureError.fromDifference(difference, actual); // No differences means we passed and there's nothing left to do. - if (error === FixtureError.None) return; + if (difference === Difference.None) return; + + const error = FixtureError.fromDifference(difference, actual); // If we're not overwriting the current values with whatever we get this time // around, then we have a legitimate error that we need to report. diff --git a/packages/babel-parser/test/helpers/to-contextual-syntax-error.js b/packages/babel-parser/test/helpers/to-contextual-syntax-error.js index ba1607280d3d..e4d179045bdc 100644 --- a/packages/babel-parser/test/helpers/to-contextual-syntax-error.js +++ b/packages/babel-parser/test/helpers/to-contextual-syntax-error.js @@ -19,15 +19,21 @@ export default function toContextualSyntaxError( { highlightCode: true }, ); - // Limit this stack trace to 1. Everything after that is just stuff from the - // test. + // We make this a lazy property since we don't want pay the price of creating + // this new error unless we are going to display it. Object.defineProperty(error, "context", { get() { const message = JSON.stringify(`${error.message}\n${frame}`); const originalStackTraceLimit = Error.stackTraceLimit; + // Limit this stack trace to 1. Everything after that is just stuff from + // the test. Error.stackTraceLimit = 1; + // The only way to manipulate the file in which a SyntaxError reports it + // is coming from is to roundtrip through vm.runInThisContext. If v8 + // supported Firefox's non-standard Error.fileName property, that could be + // used instead, but unfortunately it does not. const context = runInThisContext(`SyntaxError(${message})`, { filename }); Error.stackTraceLimit = originalStackTraceLimit; From caa9a8ceb268151389e39d213c24478a3c7a02fc Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Wed, 26 Jan 2022 14:32:45 -0800 Subject: [PATCH 28/34] Throw early if we are in CI, and make sure runFixtureText has its JSDocs comments. Reviewed by @tolmasky. --- .../test/helpers/run-fixture-tests.js | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index 2be162ff7d91..ab89676f7b1a 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -7,9 +7,23 @@ import toFuzzedOptions from "./to-fuzzed-options.js"; import { serialize, deserialize } from "./serialization.js"; import toContextualSyntaxError from "./to-contextual-syntax-error.js"; -const { OVERWRITE } = process.env; +const { CI, OVERWRITE } = process.env; const { stringify, parse: JSONParse } = JSON; +/** + * run parser on given tests + * + * @param {Test} A {@link packages/babel-helper-fixtures/src/index.js Test} + * instance + generated from `getFixtures` + * @param {*} parseFunction A parser with the same interface of + * `@babel/parser#parse` + * @param {boolean} [compareErrorsOnly=false] Whether we should only compare the + * "errors" of generated ast against the expected AST. Used for tests where an + * ESTree AST is generated but we want to make sure `@babel/parser` still throws + * expected recoverable recoverable errors on given code locations. + * @returns {void} + */ export default function runFixtureTests( fixturesPath, parseFunction, @@ -89,7 +103,7 @@ function runParseTest(parse, test, onlyCompareErrors) { // If we're not overwriting the current values with whatever we get this time // around, then we have a legitimate error that we need to report. - if (!OVERWRITE) throw error; + if (CI || !OVERWRITE) throw error; // We only write the output of the original test, not all it's auto-generated // variations. From 9b87eeaeb5ba81d228c92e8a34f236f777d96226 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Thu, 27 Jan 2022 11:34:19 -0800 Subject: [PATCH 29/34] Fix JSDocs. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/run-fixture-tests.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index ab89676f7b1a..c0c0d2671bc5 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -13,12 +13,10 @@ const { stringify, parse: JSONParse } = JSON; /** * run parser on given tests * - * @param {Test} A {@link packages/babel-helper-fixtures/src/index.js Test} - * instance - generated from `getFixtures` + * @param {string} fixturesPath A base search path for finding fixtures. * @param {*} parseFunction A parser with the same interface of * `@babel/parser#parse` - * @param {boolean} [compareErrorsOnly=false] Whether we should only compare the + * @param {boolean} [onlyCompareErrors=false] Whether we should only compare the * "errors" of generated ast against the expected AST. Used for tests where an * ESTree AST is generated but we want to make sure `@babel/parser` still throws * expected recoverable recoverable errors on given code locations. From b1118e410f39e2f73ade31720e42485de7cf793a Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Thu, 27 Jan 2022 13:45:39 -0800 Subject: [PATCH 30/34] Change fuzz testing to be opt-in while we only do line changes. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/to-fuzzed-options.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-parser/test/helpers/to-fuzzed-options.js b/packages/babel-parser/test/helpers/to-fuzzed-options.js index bc90e037eb2f..4ed5de8334ff 100644 --- a/packages/babel-parser/test/helpers/to-fuzzed-options.js +++ b/packages/babel-parser/test/helpers/to-fuzzed-options.js @@ -46,7 +46,7 @@ const toAdjustedSyntaxError = (adjust, error) => : error; export default function toFuzzedOptions(options) { - if (TEST_FUZZ === "false") return [[false, options]]; + if (TEST_FUZZ !== "true") return [[false, options]]; const { startLine = 1, startColumn = 0 } = options; From dec5379c214d89f0ee6417ef4084cd5efec4964c Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Thu, 27 Jan 2022 14:36:52 -0800 Subject: [PATCH 31/34] Don't check if file exists before deleting it, and only overwite options if there's a throw property. Reviewed by @tolmasky. --- .../test/helpers/run-fixture-tests.js | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index c0c0d2671bc5..aaab52d674b9 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -1,5 +1,5 @@ import { multiple as getFixtures } from "@babel/helper-fixtures"; -import { existsSync, readFileSync, unlinkSync, writeFileSync } from "fs"; +import { readFileSync, unlinkSync, writeFileSync } from "fs"; import { join } from "path"; import Difference from "./difference.js"; import FixtureError from "./fixture-error.js"; @@ -124,10 +124,13 @@ function runParseTest(parse, test, onlyCompareErrors) { // Store (or overwrite) the options file if there's anything to record, // otherwise remove it. - if (Object.keys(newOptions).length > 0) { + if (Object.keys(newOptions).length <= 0) { + rmf(optionsLocation); + } else if (throws) { + // The idea here is that we shouldn't need to change anything if this doesn't + // throw, and stringify will produce different output than what prettier + // wants. writeFileSync(optionsLocation, stringify(newOptions, null, 2), "utf-8"); - } else if (existsSync(optionsLocation)) { - unlinkSync(optionsLocation); } // When only comparing errors, we don't want to overwrite the AST JSON because @@ -148,8 +151,8 @@ function runParseTest(parse, test, onlyCompareErrors) { // Remove any previous output files that are no longer valid, either because // extension changed, or because we aren't writing it out at all anymore. for (const location of [normalLocation, extendedLocation]) { - if (location !== outputLocation && existsSync(location)) { - unlinkSync(location); + if (location !== outputLocation) { + rmf(location); } } } @@ -162,6 +165,12 @@ function readJSON(filename) { } } +function rmf(path) { + try { + unlinkSync(path); + } catch (error) {} +} + function parseWithRecovery(parse, source, filename, options) { try { const ast = parse(source, { errorRecovery: true, ...options }); From 198ad6bcf2a18fc394de65506ee76d751e58919e Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Thu, 27 Jan 2022 16:03:28 -0800 Subject: [PATCH 32/34] Put a newline at the end of JSON files. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/run-fixture-tests.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index aaab52d674b9..eb9fd13f4d0e 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -10,6 +10,9 @@ import toContextualSyntaxError from "./to-contextual-syntax-error.js"; const { CI, OVERWRITE } = process.env; const { stringify, parse: JSONParse } = JSON; +const writeFileWithNewline = (path, string) => + writeFileSync(path, `${string}\n`, "utf-8"); + /** * run parser on given tests * @@ -130,7 +133,7 @@ function runParseTest(parse, test, onlyCompareErrors) { // The idea here is that we shouldn't need to change anything if this doesn't // throw, and stringify will produce different output than what prettier // wants. - writeFileSync(optionsLocation, stringify(newOptions, null, 2), "utf-8"); + writeFileWithNewline(optionsLocation, stringify(newOptions, null, 2)); } // When only comparing errors, we don't want to overwrite the AST JSON because @@ -145,7 +148,7 @@ function runParseTest(parse, test, onlyCompareErrors) { serialized && (extended ? extendedLocation : normalLocation); if (outputLocation) { - writeFileSync(outputLocation, serialized, "utf-8"); + writeFileWithNewline(outputLocation, serialized); } // Remove any previous output files that are no longer valid, either because From 91f73c769cb69f4b9c6df7553046c4a65267118d Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Fri, 28 Jan 2022 08:20:33 -0800 Subject: [PATCH 33/34] Only refrain from throwing if the error is ENOENT. Also, clean up the error message when there is no cause. Reviewed by @tolmasky. --- .../babel-parser/test/helpers/fixture-error.js | 14 ++++++-------- .../babel-parser/test/helpers/run-fixture-tests.js | 4 +++- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index 94a33aeab591..787f121f41b2 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -37,14 +37,12 @@ export default class FixtureError extends Error { // Don't show the stack of FixtureErrors, it's irrelevant. // Instead, show the cause, if present. [inspect.custom](depth, options) { - return `${this.message.replace(/(?<=error(s?))\.$/, ":\n")}${ - this.cause - ? `\n${inspect(toContextError(this.cause), options)}`.replace( - /\n/g, - "\n ", - ) - : "" - }`; + return this.cause + ? `${this.message.replace(/(?<=error(s?))\.$/, ":\n")}\n${inspect( + toContextError(this.cause), + options + )}`.replace(/\n/g, "\n ") + : this.message; } static fromDifference(difference, actual) { diff --git a/packages/babel-parser/test/helpers/run-fixture-tests.js b/packages/babel-parser/test/helpers/run-fixture-tests.js index eb9fd13f4d0e..0f89bcdbb540 100644 --- a/packages/babel-parser/test/helpers/run-fixture-tests.js +++ b/packages/babel-parser/test/helpers/run-fixture-tests.js @@ -171,7 +171,9 @@ function readJSON(filename) { function rmf(path) { try { unlinkSync(path); - } catch (error) {} + } catch (error) { + if (error.code !== "ENOENT") throw error; + } } function parseWithRecovery(parse, source, filename, options) { From 4ec565885e44efb0dc22f8fad46aae90d6659434 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Fri, 28 Jan 2022 08:41:43 -0800 Subject: [PATCH 34/34] Fix linter error. Reviewed by @tolmasky. --- packages/babel-parser/test/helpers/fixture-error.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-parser/test/helpers/fixture-error.js b/packages/babel-parser/test/helpers/fixture-error.js index 787f121f41b2..0043cff02158 100644 --- a/packages/babel-parser/test/helpers/fixture-error.js +++ b/packages/babel-parser/test/helpers/fixture-error.js @@ -40,7 +40,7 @@ export default class FixtureError extends Error { return this.cause ? `${this.message.replace(/(?<=error(s?))\.$/, ":\n")}\n${inspect( toContextError(this.cause), - options + options, )}`.replace(/\n/g, "\n ") : this.message; }