From b25c14feb133ffad247cf5cb8358ecaebd75d962 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Thu, 9 Nov 2023 16:50:00 -0500 Subject: [PATCH] [tests][be] Clean up fixture selection logic --- Refactor selection logic to be easier to read; add support for .jsx test files (feels a bit weird adding a `.jsx` fixture and not seeing it get run) --- .../src/__tests__/fixtures/tsconfig.json | 1 + .../fixture-test-utils/src/fixture-utils.ts | 121 +++++++++++------- compiler/packages/snap/src/compiler-worker.ts | 7 +- compiler/packages/sprout/src/runner-worker.ts | 6 +- 4 files changed, 81 insertions(+), 54 deletions(-) diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/tsconfig.json b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/tsconfig.json index c4e800fe03..2486b61eea 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/tsconfig.json +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/tsconfig.json @@ -20,6 +20,7 @@ }, "include": [ "./compiler/**/*.js", + "./compiler/**/*.jsx", "./compiler/**/*.ts", "./compiler/**/*.tsx" ] diff --git a/compiler/packages/fixture-test-utils/src/fixture-utils.ts b/compiler/packages/fixture-test-utils/src/fixture-utils.ts index fdba8808c0..87f5b75cbc 100644 --- a/compiler/packages/fixture-test-utils/src/fixture-utils.ts +++ b/compiler/packages/fixture-test-utils/src/fixture-utils.ts @@ -7,6 +7,7 @@ import fs from "fs/promises"; import glob from "glob"; +import invariant from "invariant"; import path from "path"; import { FILTER_PATH, FIXTURES_PATH } from "./constants"; @@ -32,6 +33,35 @@ async function exists(file: string): Promise { } } +function stripExtension(filename: string, extensions: Array): string { + for (const ext of extensions) { + if (filename.endsWith(ext)) { + return filename.slice(0, -ext.length); + } + } + return filename; +} + +function shouldSkip( + filter: TestFilter | null, + filterId: string, + filename: string +) { + if (filter) { + if (filter.kind === "only" && filter.paths.indexOf(filterId) === -1) { + return true; + } else if ( + filter.kind === "skip" && + filter.paths.indexOf(filterId) !== -1 + ) { + return true; + } + } else if (filename.startsWith("todo.")) { + return true; + } + return false; +} + export async function readTestFilter(): Promise { if (!(await exists(FILTER_PATH))) { throw new Error(`testfilter file not found at ${FILTER_PATH}`); @@ -71,71 +101,70 @@ export async function readTestFilter(): Promise { export type TestFixture = { basename: string; - inputPath: string; - inputExists: boolean; + inputPath: string | null; outputPath: string; outputExists: boolean; }; +const INPUT_EXTENSIONS = [".js", ".ts", ".jsx", ".tsx"]; +const OUTPUT_EXTENSION = ".expect.md"; export function getFixtures( filter: TestFilter | null ): Map { // search for fixtures within nested directories - const files = glob.sync(`**/*.{js,ts,tsx,md}`, { + const inputFiles = glob.sync(`**/*{${INPUT_EXTENSIONS.join(",")}}`, { cwd: FIXTURES_PATH, }); const fixtures: Map = new Map(); - - for (const filePath of files) { - const basename = path.basename( - path.basename( - path.basename(path.basename(filePath, ".js"), ".ts"), - ".tsx" - ), - ".expect.md" - ); - // "partial" paths do not include suffixes - const partialRelativePath = path.join(path.dirname(filePath), basename); - // Replicate jest test behavior - if (basename.startsWith("todo.")) { + for (const filePath of inputFiles) { + const filename = path.basename(filePath); + // Do not include extensions in unique identifier for fixture + const partialPath = stripExtension(filePath, INPUT_EXTENSIONS); + if (shouldSkip(filter, partialPath, filename)) { continue; } - if (filter) { - if ( - filter.kind === "only" && - filter.paths.indexOf(partialRelativePath) === -1 - ) { - continue; - } else if ( - filter.kind === "skip" && - filter.paths.indexOf(partialRelativePath) !== -1 - ) { - continue; - } - } - let fixtureInfo = fixtures.get(partialRelativePath); + const fixtureInfo = fixtures.get(partialPath); if (fixtureInfo === undefined) { - const partialAbsolutePath = path.join(FIXTURES_PATH, partialRelativePath); - fixtureInfo = { - basename, - inputPath: `${partialAbsolutePath}.js`, - inputExists: false, - outputPath: `${partialAbsolutePath}.expect.md`, + fixtures.set(partialPath, { + basename: path.basename(partialPath), + inputPath: path.join(FIXTURES_PATH, filePath), + outputPath: path.join(FIXTURES_PATH, partialPath) + OUTPUT_EXTENSION, outputExists: false, - }; - fixtures.set(partialRelativePath, fixtureInfo); + }); + } else { + console.warn( + "Found duplicate fixture files: ", + fixtureInfo.inputPath, + filePath + ); + } + } + + const outputFiles = glob.sync(`**/*${OUTPUT_EXTENSION}`, { + cwd: FIXTURES_PATH, + }); + for (const filePath of outputFiles) { + const filename = path.basename(filePath); + // Do not include extensions in unique identifier for fixture + const partialPath = stripExtension(filePath, [OUTPUT_EXTENSION]); + if (shouldSkip(filter, partialPath, filename)) { + continue; } - if ( - filePath.endsWith(".js") || - filePath.endsWith(".ts") || - filePath.endsWith(".tsx") - ) { - // inputPath may have a different file extension than the .js default - fixtureInfo.inputPath = path.join(FIXTURES_PATH, filePath); - fixtureInfo.inputExists = true; + const fixtureInfo = fixtures.get(partialPath); + if (fixtureInfo === undefined) { + fixtures.set(partialPath, { + basename: path.basename(partialPath), + inputPath: null, + outputPath: path.join(FIXTURES_PATH, filePath), + outputExists: true, + }); } else { + invariant( + fixtureInfo.outputPath === path.join(FIXTURES_PATH, filePath), + "Unexpected output filepath" + ); fixtureInfo.outputExists = true; } } diff --git a/compiler/packages/snap/src/compiler-worker.ts b/compiler/packages/snap/src/compiler-worker.ts index 164c0273aa..488e6b7596 100644 --- a/compiler/packages/snap/src/compiler-worker.ts +++ b/compiler/packages/snap/src/compiler-worker.ts @@ -30,7 +30,6 @@ export function clearRequireCache() { } export type TestResult = { - inputPath: string; outputPath: string; actual: string | null; // null == input did not exist expected: string | null; // null == output did not exist @@ -54,16 +53,15 @@ export async function compile( clearRequireCache(); } version = compilerVersion; - const { inputPath, inputExists, outputPath, outputExists, basename } = + const { inputPath, outputPath, outputExists, basename } = fixture; - const input = inputExists ? await fs.readFile(inputPath, "utf8") : null; + const input = inputPath != null ? await fs.readFile(inputPath, "utf8") : null; const expected = outputExists ? await fs.readFile(outputPath, "utf8") : null; // Input will be null if the input file did not exist, in which case the output file // is stale if (input === null) { return { - inputPath, outputPath, actual: null, expected, @@ -128,7 +126,6 @@ export async function compile( console.error = originalConsoleError; return { - inputPath, outputPath, actual: output, expected, diff --git a/compiler/packages/sprout/src/runner-worker.ts b/compiler/packages/sprout/src/runner-worker.ts index be77232fd8..04d4aec18d 100644 --- a/compiler/packages/sprout/src/runner-worker.ts +++ b/compiler/packages/sprout/src/runner-worker.ts @@ -191,12 +191,12 @@ export async function run(fixture: TestFixture): Promise { console.error = (...messages: Array) => { seenConsoleErrors.push(...messages); }; - const { inputPath, inputExists } = fixture; - if (!inputExists) { + const { inputPath } = fixture; + if (inputPath == null) { return { nonForgetResult: null, forgetResult: null, - unexpectedError: "file did not exist!", + unexpectedError: "No input for fixture " + fixture.outputPath, }; } const inputRaw = await fs.readFile(inputPath, "utf8");