From 370844cc4cbe3c43b8e699e3e4bcec215bd94e08 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 4 Sep 2020 23:43:41 +0800 Subject: [PATCH] Run full test on every parser (#9070) * Refactor test * Fix * Improve `hasPragma` & `insertPragma` on files has BOM * Fix `coreFormat` cursorOffset * Code style * Update snapshots * Refactor * Restore BOM test * Add export * Empty file * return after SyntaxError * EOL test * Fix eol tests * Named export * Tweak matrix * Disable on `requirePragma` * Disable `cr` test * Simplify logic * Update `verbose` * Remove `test:full-test` * Typo --- .github/workflows/dev-test.yml | 4 + .github/workflows/prod-test.yml | 6 + CONTRIBUTING.md | 9 +- jest.config.js | 1 + tests/js/eol/__snapshots__/jsfmt.spec.js.snap | 4 - tests_config/run_spec.js | 318 ++++++++++-------- 6 files changed, 186 insertions(+), 156 deletions(-) diff --git a/.github/workflows/dev-test.yml b/.github/workflows/dev-test.yml index 93fa4bc43..cea004c56 100644 --- a/.github/workflows/dev-test.yml +++ b/.github/workflows/dev-test.yml @@ -26,6 +26,8 @@ jobs: - os: "ubuntu-latest" node: "14" ENABLE_CODE_COVERAGE: true + FULL_TEST: true + CHECK_TEST_PARSERS: true exclude: - os: "macos-latest" node: "12" @@ -33,6 +35,8 @@ jobs: node: "12" env: ENABLE_CODE_COVERAGE: ${{ matrix.ENABLE_CODE_COVERAGE }} + FULL_TEST: ${{ matrix.FULL_TEST }} + CHECK_TEST_PARSERS: ${{ matrix.CHECK_TEST_PARSERS }} name: Node.js ${{ matrix.node }} on ${{ matrix.os }} runs-on: ${{ matrix.os }} steps: diff --git a/.github/workflows/prod-test.yml b/.github/workflows/prod-test.yml index d215e23d4..9c72010c0 100644 --- a/.github/workflows/prod-test.yml +++ b/.github/workflows/prod-test.yml @@ -87,11 +87,17 @@ jobs: - "14" - "12" - "10" + include: + - os: "ubuntu-latest" + node: "14" + FULL_TEST: true exclude: - os: "macos-latest" node: "12" - os: "windows-latest" node: "12" + env: + FULL_TEST: ${{ matrix.FULL_TEST }} name: Node.js ${{ matrix.node }} on ${{ matrix.os }} runs-on: ${{ matrix.os }} needs: [build] diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 34a84e200..f5c227dd7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,9 +12,12 @@ Here's what you need to know about the tests: - The tests use [Jest snapshots](https://facebook.github.io/jest/docs/en/snapshot-testing.html). - You can make changes and run `jest -u` (or `yarn test -u`) to update the snapshots. Then run `git diff` to take a look at what changed. Always update the snapshots when opening a PR. -- You can run `AST_COMPARE=1 DEEP_COMPARE=1 jest` for a more robust test run. - - `AST_COMPARE` That formats each file, re-parses it, and compares the new AST with the original one and makes sure they are semantically equivalent. - - `DEEP_COMPARE` That formats each file, then formats the output again, and checks that the second output is the same as the first. +- You can run `FULL_TEST=1 jest` for a more robust test run. + - "compare AST" That re-parses the output, and compares the new AST with the original one and makes sure they are semantically equivalent. + - "second format" That formats the output again, and checks that the second output is the same as the first. + - "EOL "\r\n"" That replace end of line with `\r\n`, and checks that the output is the same as the first. + - "EOL "\r"" That replace end of line with `\r`, and checks that the output is the same as the first. + - "BOM" That add BOM(`U+FEFF`) to text, and checks that the output is the same as the first, and keeps the BOM. - Each test folder has a `jsfmt.spec.js` that runs the tests. For JavaScript files, generally you can just put `run_spec(__dirname, ["babel", "flow", "typescript"]);` there. This will verify that the output using each parser is the same. You can also pass options as the third argument, like this: `run_spec(__dirname, ["babel"], { trailingComma: "es5" });` - `tests/flow-repo/` contains the Flow test suite, and is not supposed to be edited by hand. To update it, clone the Flow repo next to the Prettier repo and run: `node scripts/sync-flow-tests.js ../flow/tests/`. - If you would like to debug prettier locally, you can either debug it in Node or the browser. The easiest way to debug it in Node, is to create a local test file with some example code you want formatted and either run it in an editor like VS Code or run it directly via `./bin/prettier.js `. The easiest way to debug it in the browser is to build Prettier's website locally (see `website/README.md`). diff --git a/jest.config.js b/jest.config.js index 638efec88..330cf4d54 100644 --- a/jest.config.js +++ b/jest.config.js @@ -33,4 +33,5 @@ module.exports = { "jest-watch-typeahead/filename", "jest-watch-typeahead/testname", ], + verbose: Boolean(process.env.FULL_TEST), }; diff --git a/tests/js/eol/__snapshots__/jsfmt.spec.js.snap b/tests/js/eol/__snapshots__/jsfmt.spec.js.snap index 7f31a623f..c8d4614c1 100644 --- a/tests/js/eol/__snapshots__/jsfmt.spec.js.snap +++ b/tests/js/eol/__snapshots__/jsfmt.spec.js.snap @@ -10,7 +10,6 @@ printWidth: 80 (function(){return <|>15})() =====================================output===================================== -** The EOL of output might not real when \`TEST_CRLF\` ** (function () { return <|>15; })(); @@ -86,7 +85,6 @@ printWidth: 80 6 | 7 | let x =====================================output===================================== -** The EOL of output might not real when \`TEST_CRLF\` ** class a { @@ -196,7 +194,6 @@ printWidth: 80 7 | let x 8 | =====================================output===================================== -** The EOL of output might not real when \`TEST_CRLF\` ** class a { @@ -314,7 +311,6 @@ printWidth: 80 8 | let x 9 | =====================================output===================================== -** The EOL of output might not real when \`TEST_CRLF\` ** diff --git a/tests_config/run_spec.js b/tests_config/run_spec.js index cca8cdaca..3a559fb6e 100644 --- a/tests_config/run_spec.js +++ b/tests_config/run_spec.js @@ -4,7 +4,6 @@ const { TEST_STANDALONE } = process.env; const fs = require("fs"); const path = require("path"); -const { isCI } = require("ci-info"); const prettier = !TEST_STANDALONE ? require("prettier/local") : require("prettier/standalone"); @@ -16,10 +15,8 @@ const visualizeEndOfLine = require("./utils/visualize-end-of-line"); const consistentEndOfLine = require("./utils/consistent-end-of-line"); const stringifyOptionsForTitle = require("./utils/stringify-options-for-title"); -const AST_COMPARE = isCI || process.env.AST_COMPARE; -const DEEP_COMPARE = isCI || process.env.DEEP_COMPARE; -const TEST_CRLF = - (isCI && process.platform === "win32") || process.env.TEST_CRLF; +const { FULL_TEST } = process.env; +const BOM = "\uFEFF"; const CURSOR_PLACEHOLDER = "<|>"; const RANGE_START_PLACEHOLDER = "<<>>"; @@ -61,7 +58,7 @@ const isUnstable = (filename, options) => { return testFunction(options); }; -const shouldThrowOnVerify = (filename, options) => { +const shouldThrowOnFormat = (filename, options) => { if (options.parser !== "babel-ts") { return false; } @@ -83,8 +80,8 @@ const isTestDirectory = (dirname, name) => dirname.startsWith(path.join(__dirname, "../tests", name)); function runSpec(fixtures, parsers, options) { - fixtures = typeof fixtures === "string" ? { dirname: fixtures } : fixtures; - const { dirname } = fixtures; + let { dirname, snippets = [] } = + typeof fixtures === "string" ? { dirname: fixtures } : fixtures; // `IS_PARSER_INFERENCE_TESTS` mean to test `inferParser` on `standalone` const IS_PARSER_INFERENCE_TESTS = isTestDirectory( @@ -98,12 +95,10 @@ function runSpec(fixtures, parsers, options) { const IS_ERROR_TESTS = isTestDirectory(dirname, "misc/errors"); if (IS_PARSER_INFERENCE_TESTS) { - parsers = []; - } else if (!parsers || !parsers.length) { - throw new Error(`No parsers were specified for ${dirname}`); + parsers = [undefined]; } - const snippets = (fixtures.snippets || []).map((test, index) => { + snippets = snippets.map((test, index) => { test = typeof test === "string" ? { code: test } : test; return { ...test, @@ -136,175 +131,200 @@ function runSpec(fixtures, parsers, options) { .filter(Boolean); // Make sure tests are in correct location - // only runs on local and one task on CI - if (!isCI || process.env.ENABLE_CODE_COVERAGE) { + if (process.env.CHECK_TEST_PARSERS) { + if (!Array.isArray(parsers) || !parsers.length) { + throw new Error(`No parsers were specified for ${dirname}`); + } checkParsers({ dirname, files }, parsers); } - const stringifiedOptions = stringifyOptionsForTitle(options); - const [parser, ...verifyParsers] = parsers; - + const [parser] = parsers; + const allParsers = [...parsers]; if (parsers.includes("typescript") && !parsers.includes("babel-ts")) { - verifyParsers.push("babel-ts"); + allParsers.push("babel-ts"); } + const stringifiedOptions = stringifyOptionsForTitle(options); + for (const { name, filename, code, output } of [...files, ...snippets]) { - describe(`${name}${ + const title = `${name}${ stringifiedOptions ? ` - ${stringifiedOptions}` : "" - }`, () => { + }`; + + describe(title, () => { const formatOptions = { printWidth: 80, ...options, filepath: filename, parser, }; + const formatWithMainParser = () => format(code, formatOptions); if (IS_ERROR_TESTS) { test("error test", () => { - expect(() => { - format(code, formatOptions); - }).toThrowErrorMatchingSnapshot(); + expect(formatWithMainParser).toThrowErrorMatchingSnapshot(); }); return; } - const eolReplacedCode = TEST_CRLF ? code.replace(/\n/g, "\r\n") : code; - const firstFormat = format(eolReplacedCode, formatOptions); + const mainParserFormatResult = formatWithMainParser(); - test("format", () => { - // Make sure output has consistent EOL - expect(firstFormat.eolVisualizedOutput).toEqual( - visualizeEndOfLine(consistentEndOfLine(firstFormat.outputWithCursor)) - ); + for (const currentParser of allParsers) { + runTest({ + parsers, + name, + filename, + code, + output, + parser: currentParser, + mainParserFormatResult, + mainParserFormatOptions: formatOptions, + }); + } + }); + } +} - if (typeof output === "string") { - expect(firstFormat.output).toEqual(output); - return; - } +function runTest({ + parsers, + name, + filename, + code, + output, + parser, + mainParserFormatResult, + mainParserFormatOptions, +}) { + let formatOptions = mainParserFormatOptions; + let formatResult = mainParserFormatResult; + let formatTestTitle = "format"; - const hasEndOfLine = "endOfLine" in formatOptions; - let codeForSnapshot = hasEndOfLine - ? code - .replace(RANGE_START_PLACEHOLDER, "") - .replace(RANGE_END_PLACEHOLDER, "") - : firstFormat.inputWithCursor; - let codeOffset = 0; - let resultForSnapshot = firstFormat.outputWithCursor; - const { rangeStart, rangeEnd } = firstFormat.options; + // Verify parsers + if (parser !== parsers[0]) { + formatOptions = { ...mainParserFormatResult.options, parser }; + const runFormat = () => format(code, formatOptions); - if (typeof rangeStart === "number" || typeof rangeEnd === "number") { - if (TEST_CRLF && hasEndOfLine) { - codeForSnapshot = codeForSnapshot.replace(/\n/g, "\r\n"); - } - codeForSnapshot = visualizeRange(codeForSnapshot, { - rangeStart, - rangeEnd, - }); - codeOffset = codeForSnapshot.match(/^>?\s+1 \| /)[0].length; - } - - // When `TEST_CRLF` and `endOfLine: "auto"`, the eol is always `\r\n`, - // Replace it with guess result from original input - let resultNote = ""; - if (formatOptions.endOfLine === "auto") { - const { - guessEndOfLine, - convertEndOfLineToChars, - } = require("../src/common/end-of-line"); - const originalAutoResult = convertEndOfLineToChars( - guessEndOfLine(code) - ); - if (originalAutoResult !== "\r\n") { - resultNote += - "** The EOL of output might not real when `TEST_CRLF` **"; - - if (TEST_CRLF) { - resultForSnapshot = resultForSnapshot.replace( - /\r\n?/g, - originalAutoResult - ); - } - } - } - - if (hasEndOfLine) { - codeForSnapshot = visualizeEndOfLine(codeForSnapshot); - resultForSnapshot = visualizeEndOfLine(resultForSnapshot); - } - - if (resultNote) { - resultForSnapshot = `${resultNote}\n${resultForSnapshot}`; - } - - expect( - createSnapshot( - codeForSnapshot, - resultForSnapshot, - composeOptionsForSnapshot(firstFormat.options, parsers), - { codeOffset } - ) - ).toMatchSnapshot(); + if (shouldThrowOnFormat(name, formatOptions)) { + test(`[${parser}] expect SyntaxError`, () => { + expect(runFormat).toThrow(TEST_STANDALONE ? undefined : SyntaxError); }); + return; + } - for (const parser of verifyParsers) { - const verifyOptions = { ...firstFormat.options, parser }; - const runVerifyFormat = () => format(firstFormat.input, verifyOptions); + // Verify parsers format result should be the same as main parser + output = mainParserFormatResult.outputWithCursor; + formatResult = runFormat(); + formatTestTitle = `[${parser}] format`; + } - if (shouldThrowOnVerify(name, verifyOptions)) { - test(`verify (${parser})`, () => { - expect(runVerifyFormat).toThrow( - TEST_STANDALONE ? undefined : SyntaxError - ); - }); - } else { - const verifyFormat = runVerifyFormat(); - test(`verify (${parser})`, () => { - expect(verifyFormat.eolVisualizedOutput).toEqual( - firstFormat.eolVisualizedOutput - ); - }); + test(formatTestTitle, () => { + // Make sure output has consistent EOL + expect(formatResult.eolVisualizedOutput).toEqual( + visualizeEndOfLine(consistentEndOfLine(formatResult.outputWithCursor)) + ); - if (AST_COMPARE && verifyFormat.changed && code.trim()) { - test(`verify AST (${parser})`, () => { - const originalAst = parse(verifyFormat.input, verifyOptions); - const formattedAst = parse(verifyFormat.output, verifyOptions); - expect(originalAst).toEqual(formattedAst); - }); - } - } + // The result is assert to equals to `output` + if (typeof output === "string") { + expect(formatResult.eolVisualizedOutput).toEqual( + visualizeEndOfLine(output) + ); + return; + } + + // All parsers have the same result, only snapshot the result from main parser + // TODO: move this part to `createSnapshot` + const hasEndOfLine = "endOfLine" in formatOptions; + let codeForSnapshot = formatResult.inputWithCursor; + let codeOffset = 0; + let resultForSnapshot = formatResult.outputWithCursor; + const { rangeStart, rangeEnd } = formatResult.options; + + if (typeof rangeStart === "number" || typeof rangeEnd === "number") { + codeForSnapshot = visualizeRange(codeForSnapshot, { + rangeStart, + rangeEnd, + }); + codeOffset = codeForSnapshot.match(/^>?\s+1 \| /)[0].length; + } + + if (hasEndOfLine) { + codeForSnapshot = visualizeEndOfLine(codeForSnapshot); + resultForSnapshot = visualizeEndOfLine(resultForSnapshot); + } + + expect( + createSnapshot( + codeForSnapshot, + resultForSnapshot, + composeOptionsForSnapshot(formatResult.options, parsers), + { codeOffset } + ) + ).toMatchSnapshot(); + }); + + if (!FULL_TEST) { + return; + } + + const isUnstableTest = isUnstable(filename, formatOptions); + if ( + (formatResult.changed || isUnstableTest) && + // No range and cursor + formatResult.input === code + ) { + test(`[${parser}] second format`, () => { + const { eolVisualizedOutput: firstOutput, output } = formatResult; + const { eolVisualizedOutput: secondOutput } = format( + output, + formatOptions + ); + if (isUnstableTest) { + // To keep eye on failed tests, this assert never supposed to pass, + // if it fails, just remove the file from `unstableTests` + expect(secondOutput).not.toEqual(firstOutput); + } else { + expect(secondOutput).toEqual(firstOutput); } + }); + } - const isUnstableTest = isUnstable(filename, formatOptions); - if ( - DEEP_COMPARE && - (firstFormat.changed || isUnstableTest) && - // No range and cursor - firstFormat.input === eolReplacedCode - ) { - test("second format", () => { - const { eolVisualizedOutput: firstOutput, output } = firstFormat; - const { eolVisualizedOutput: secondOutput } = format( - output, - formatOptions - ); - if (isUnstableTest) { - // To keep eye on failed tests, this assert never supposed to pass, - // if it fails, just remove the file from `unstableTests` - expect(secondOutput).not.toEqual(firstOutput); - } else { - expect(secondOutput).toEqual(firstOutput); - } - }); - } + // Some parsers skip parsing empty files + if (formatResult.changed && code.trim()) { + test(`[${parser}] compare AST`, () => { + const { input, output } = formatResult; + const originalAst = parse(input, formatOptions); + const formattedAst = parse(output, formatOptions); + expect(formattedAst).toEqual(originalAst); + }); + } - if (AST_COMPARE && firstFormat.changed) { - test("compare AST", () => { - const { input, output } = firstFormat; - const originalAst = parse(input, formatOptions); - const formattedAst = parse(output, formatOptions); - expect(formattedAst).toEqual(originalAst); - }); - } + if (!code.includes("\r") && !formatOptions.requirePragma) { + for (const eol of [ + "\r\n", + // There are some edge cases failed on `\r` test, disable for now + // "\r" + ]) { + test(`[${parser}] EOL ${JSON.stringify(eol)}`, () => { + const output = format(code.replace(/\n/g, eol), formatOptions) + .eolVisualizedOutput; + // Only if `endOfLine: "auto"` the result will be different + const expected = + formatOptions.endOfLine === "auto" + ? visualizeEndOfLine( + // All `code` use `LF`, so the `eol` of result is always `LF` + formatResult.outputWithCursor.replace(/\n/g, eol) + ) + : formatResult.eolVisualizedOutput; + expect(output).toEqual(expected); + }); + } + } + + if (code.charAt(0) !== BOM) { + test(`[${parser}] BOM`, () => { + const output = format(BOM + code, formatOptions).eolVisualizedOutput; + const expected = BOM + formatResult.eolVisualizedOutput; + expect(output).toEqual(expected); }); } }