Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions change/change-98c52c45-9c3f-4c30-9fca-4f6e22e36730.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
4 changes: 2 additions & 2 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
59 changes: 29 additions & 30 deletions packages/cli/src/commands/createReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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,
Expand All @@ -45,39 +45,38 @@ export async function createReporter(
case "verboseFileLog":
case "vfl":
return new VerboseFileLogReporter(logFile);
}

default:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be viewed with ignore whitespace--it's just moving this outside the switch and changing the indentation

// 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 });
}
27 changes: 22 additions & 5 deletions packages/cli/src/commands/initializeReporters.ts
Original file line number Diff line number Diff line change
@@ -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<string, string> = {}) {
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);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/cli/src/commands/options.ts
Original file line number Diff line number Diff line change
@@ -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"),
reporter: new Option("--reporter <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 <level>", "log level").choices(["info", "warn", "error", "verbose", "silly"]).conflicts("verbose"),
Expand Down
20 changes: 19 additions & 1 deletion packages/cli/src/types/ReporterInitOptions.ts
Original file line number Diff line number Diff line change
@@ -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<BuiltInReporterName, true>);

export interface ReporterInitOptions {
reporter: string[] | string;
reporter: ReporterName[] | ReporterName | undefined;
progress: boolean;
verbose: boolean;
grouped: boolean;
Expand Down
Loading