diff --git a/change/change-98c52c45-9c3f-4c30-9fca-4f6e22e36730.json b/change/change-98c52c45-9c3f-4c30-9fca-4f6e22e36730.json new file mode 100644 index 000000000..573f3d527 --- /dev/null +++ b/change/change-98c52c45-9c3f-4c30-9fca-4f6e22e36730.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "type": "patch", + "comment": "Better validation and docs of reporter names", + "packageName": "@lage-run/cli", + "email": "elcraig@microsoft.com", + "dependentChangeType": "patch" + } + ] +} \ No newline at end of file diff --git a/packages/cli/package.json b/packages/cli/package.json index f457f94d6..e62adac10 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -15,8 +15,8 @@ "main": "lib/index.js", "types": "lib/index.d.ts", "scripts": { - "build": "tsc", - "start": "tsc -w --preserveWatchOutput", + "build": "yarn run -T tsc", + "start": "yarn run -T tsc -w --preserveWatchOutput", "test": "monorepo-scripts jest", "lint": "monorepo-scripts lint" }, diff --git a/packages/cli/src/commands/createReporter.ts b/packages/cli/src/commands/createReporter.ts index fc005830b..2be60196e 100644 --- a/packages/cli/src/commands/createReporter.ts +++ b/packages/cli/src/commands/createReporter.ts @@ -7,7 +7,7 @@ import { VerboseFileLogReporter, ChromeTraceEventsReporter, } from "@lage-run/reporters"; -import type { ReporterInitOptions } from "../types/ReporterInitOptions.js"; +import type { BuiltInReporterName, ReporterInitOptions } from "../types/ReporterInitOptions.js"; import type { Reporter } from "@lage-run/logger"; import { findPackageRoot } from "workspace-tools"; import { readFileSync } from "fs"; @@ -26,7 +26,7 @@ export async function createReporter( const packageJson = JSON.parse(readFileSync(path.join(root, "package.json"), "utf-8")); const version = packageJson.version; - switch (reporter) { + switch (reporter as BuiltInReporterName) { case "profile": return new ChromeTraceEventsReporter({ concurrency, @@ -45,39 +45,38 @@ export async function createReporter( case "verboseFileLog": case "vfl": return new VerboseFileLogReporter(logFile); + } - default: - // Check if it's a custom reporter defined in config - if (customReporters && customReporters[reporter]) { - const reporterPath = customReporters[reporter]; - const resolvedPath = path.isAbsolute(reporterPath) ? reporterPath : path.resolve(process.cwd(), reporterPath); - - try { - // Use dynamic import to load the custom reporter module - // This works with both ESM (.mjs, .js with type: module) and CommonJS (.cjs, .js) files - const reporterModule = await import(pathToFileURL(resolvedPath).href); + // Check if it's a custom reporter defined in config + if (customReporters && customReporters[reporter]) { + const reporterPath = customReporters[reporter]; + const resolvedPath = path.isAbsolute(reporterPath) ? reporterPath : path.resolve(process.cwd(), reporterPath); - // Try different export patterns - const ReporterClass = reporterModule.default ?? reporterModule[reporter] ?? reporterModule; + try { + // Use dynamic import to load the custom reporter module + // This works with both ESM (.mjs, .js with type: module) and CommonJS (.cjs, .js) files + const reporterModule = await import(pathToFileURL(resolvedPath).href); - if (typeof ReporterClass === "function") { - return new ReporterClass(options); - } else if (typeof ReporterClass === "object" && ReporterClass !== null) { - // If it's already an instance - return ReporterClass; - } else { - throw new Error(`Custom reporter "${reporter}" at "${resolvedPath}" does not export a valid reporter class or instance.`); - } - } catch (error) { - throw new Error(`Failed to load custom reporter "${reporter}" from "${resolvedPath}": ${error}`); - } - } + // Try different export patterns + const ReporterClass = reporterModule.default ?? reporterModule[reporter] ?? reporterModule; - // Default reporter behavior - if (progress && !(logLevel >= LogLevel.verbose || verbose || grouped)) { - return new ProgressReporter({ concurrency, version }); + if (typeof ReporterClass === "function") { + return new ReporterClass(options); + } else if (typeof ReporterClass === "object" && ReporterClass !== null) { + // If it's already an instance + return ReporterClass; + } else { + throw new Error(`Custom reporter "${reporter}" at "${resolvedPath}" does not export a valid reporter class or instance.`); } + } catch (error) { + throw new Error(`Failed to load custom reporter "${reporter}" from "${resolvedPath}": ${error}`); + } + } - return new LogReporter({ grouped, logLevel: verbose ? LogLevel.verbose : logLevel }); + // Default reporter behavior + if (progress && !(logLevel >= LogLevel.verbose || verbose || grouped)) { + return new ProgressReporter({ concurrency, version }); } + + return new LogReporter({ grouped, logLevel: verbose ? LogLevel.verbose : logLevel }); } diff --git a/packages/cli/src/commands/initializeReporters.ts b/packages/cli/src/commands/initializeReporters.ts index e8c0aa924..b5d08e40e 100644 --- a/packages/cli/src/commands/initializeReporters.ts +++ b/packages/cli/src/commands/initializeReporters.ts @@ -1,24 +1,41 @@ import { createReporter } from "./createReporter.js"; import type { Logger } from "@lage-run/logger"; -import type { ReporterInitOptions } from "../types/ReporterInitOptions.js"; +import { + type BuiltInReporterName, + type ReporterInitOptions, + type ReporterName, + builtInReporterNames, +} from "../types/ReporterInitOptions.js"; export async function initializeReporters(logger: Logger, options: ReporterInitOptions, customReporters: Record = {}) { const { reporter } = options; + const supportedReportersLower = Object.fromEntries( + [...builtInReporterNames, ...Object.keys(customReporters)].map((name) => [name.toLowerCase(), name]) + ); + // filter out falsy values (e.g. undefined) from the reporter array - const reporterOptions = (Array.isArray(reporter) ? reporter : [reporter]).filter(Boolean); + const reporterOptions = (Array.isArray(reporter) ? reporter : [reporter]).filter(Boolean) as ReporterName[]; if (reporterOptions.length === 0) { // "default" is just a dummy name to trigger the default case in createReporter - reporterOptions.push("default"); + reporterOptions.push("default" satisfies BuiltInReporterName); } // add profile reporter if --profile is passed if (options.profile) { - reporterOptions.push("profile"); + reporterOptions.push("profile" satisfies BuiltInReporterName); } - for (const reporterName of reporterOptions) { + for (const rawReporterName of reporterOptions) { + // Validate the given name, but be flexible about the casing + const reporterName = supportedReportersLower[rawReporterName.toLowerCase()]; + if (!reporterName) { + throw new Error( + `Invalid --reporter option: "${rawReporterName}". Supported reporters are: ${Object.keys(supportedReportersLower).join(", ")}` + ); + } + const reporterInstance = await createReporter(reporterName, options, customReporters); logger.addReporter(reporterInstance); } diff --git a/packages/cli/src/commands/options.ts b/packages/cli/src/commands/options.ts index 7b0977d48..ffe2450bc 100644 --- a/packages/cli/src/commands/options.ts +++ b/packages/cli/src/commands/options.ts @@ -1,10 +1,13 @@ import { Option } from "commander"; +import { builtInReporterNames } from "../types/ReporterInitOptions.js"; const isCI = process.env.CI || process.env.TF_BUILD; +const reporterChoices = builtInReporterNames.filter((n) => n !== "default" && n !== "profile"); + const options = { logger: { - reporter: new Option("--reporter ", "reporter"), + reporter: new Option("--reporter ", `log reporter (built-in choices: ${reporterChoices.join(", ")})`), grouped: new Option("--grouped", "groups the logs").default(false), progress: new Option("--progress").conflicts(["reporter", "grouped", "verbose"]).default(!isCI), logLevel: new Option("--log-level ", "log level").choices(["info", "warn", "error", "verbose", "silly"]).conflicts("verbose"), diff --git a/packages/cli/src/types/ReporterInitOptions.ts b/packages/cli/src/types/ReporterInitOptions.ts index ef6b45611..a10ab22e5 100644 --- a/packages/cli/src/types/ReporterInitOptions.ts +++ b/packages/cli/src/types/ReporterInitOptions.ts @@ -1,7 +1,25 @@ import type { LogLevel } from "@lage-run/logger"; +/** All the built-in reporter names */ +export type BuiltInReporterName = "default" | "profile" | "json" | "azureDevops" | "adoLog" | "npmLog" | "old" | "verboseFileLog" | "vfl"; +/** Built-in or custom reporter name */ +export type ReporterName = BuiltInReporterName | string; + +export const builtInReporterNames = Object.keys({ + json: true, + azureDevops: true, + npmLog: true, + verboseFileLog: true, + vfl: true, + adoLog: true, + old: true, + default: true, + profile: true, + // This verifies all reporters are listed +} satisfies Record); + export interface ReporterInitOptions { - reporter: string[] | string; + reporter: ReporterName[] | ReporterName | undefined; progress: boolean; verbose: boolean; grouped: boolean;