Skip to content

Commit

Permalink
Improve our error messages when tests fail (#896)
Browse files Browse the repository at this point in the history
* Add better error reporting for tests.

* Control the streams that a logger writes to

* In tests, write a debug log of events and print it when a test fails

* Fix a couple little things.

* Rename uvu-timeout.ts to rig-test.ts

* Rename env variable to WIREIT_DEBUG_LOG_FILE
  • Loading branch information
rictic authored Oct 12, 2023
1 parent 9481a2c commit ab17457
Show file tree
Hide file tree
Showing 38 changed files with 1,215 additions and 1,087 deletions.
4 changes: 4 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,9 @@ module.exports = {
// maybe we should turn this on, but it started suddenly triggering in
// an unrelated change.
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/no-unused-vars': [
'error',
{argsIgnorePattern: '^_', varsIgnorePattern: '^_'},
],
},
};
37 changes: 27 additions & 10 deletions src/cli-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {MetricsLogger} from './logging/metrics-logger.js';
import {ScriptReference} from './config.js';
import {FailureMode} from './executor.js';
import {unreachable} from './util/unreachable.js';
import {Logger} from './logging/logger.js';
import {Console, Logger} from './logging/logger.js';
import {QuietCiLogger, QuietLogger} from './logging/quiet-logger.js';
import {DefaultLogger} from './logging/default-logger.js';

Expand Down Expand Up @@ -58,7 +58,7 @@ export interface Options {
logger: Logger;
}

export const getOptions = (): Result<Options> => {
export const getOptions = async (): Promise<Result<Options>> => {
// This environment variable is set by npm, yarn, and pnpm, and tells us which
// script is running.
const scriptName = process.env.npm_lifecycle_event;
Expand Down Expand Up @@ -185,26 +185,27 @@ export const getOptions = (): Result<Options> => {

const agent = getNpmUserAgent();

const console = new Console(process.stdout, process.stderr);
const packageRoot = packageDir ?? process.cwd();
const loggerResult = ((): Result<Logger> => {
const packageRoot = packageDir ?? process.cwd();
const str = process.env['WIREIT_LOGGER'];
if (!str) {
if (process.env.CI) {
return {ok: true, value: new QuietCiLogger(packageRoot)};
return {ok: true, value: new QuietCiLogger(packageRoot, console)};
}
return {ok: true, value: new QuietLogger(packageRoot)};
return {ok: true, value: new QuietLogger(packageRoot, console)};
}
if (str === 'quiet') {
return {ok: true, value: new QuietLogger(packageRoot)};
return {ok: true, value: new QuietLogger(packageRoot, console)};
}
if (str === 'quiet-ci') {
return {ok: true, value: new QuietCiLogger(packageRoot)};
return {ok: true, value: new QuietCiLogger(packageRoot, console)};
}
if (str === 'simple') {
return {ok: true, value: new DefaultLogger(packageRoot)};
return {ok: true, value: new DefaultLogger(packageRoot, console)};
}
if (str === 'metrics') {
return {ok: true, value: new MetricsLogger(packageRoot)};
return {ok: true, value: new MetricsLogger(packageRoot, console)};
}
return {
ok: false,
Expand All @@ -222,6 +223,22 @@ export const getOptions = (): Result<Options> => {
return loggerResult;
}

let logger = loggerResult.value;
if (process.env['WIREIT_DEBUG_LOG_FILE']) {
const [{DebugLogger}, {CombinationLogger}] = await Promise.all([
import('./logging/debug-logger.js'),
import('./logging/combination-logger.js'),
]);
const debugLogStream = await fs.createWriteStream(
process.env['WIREIT_DEBUG_LOG_FILE']!,
);
const debugLogConsole = new Console(debugLogStream, debugLogStream, true);
logger = new CombinationLogger(
[logger, new DebugLogger(packageRoot, debugLogConsole)],
console,
);
}

return {
ok: true,
value: {
Expand All @@ -230,7 +247,7 @@ export const getOptions = (): Result<Options> => {
cache: cacheResult.value,
failureMode: failureModeResult.value,
agent,
logger: loggerResult.value,
logger,
...getArgvOptions(script, agent),
},
};
Expand Down
6 changes: 4 additions & 2 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {unreachable} from './util/unreachable.js';
import {Failure} from './event.js';
import {packageDir, getOptions, Options} from './cli-options.js';
import {DefaultLogger} from './logging/default-logger.js';
import {Console} from './logging/logger.js';

const run = async (options: Options): Promise<Result<void, Failure[]>> => {
using logger = options.logger;
Expand Down Expand Up @@ -116,11 +117,12 @@ const run = async (options: Options): Promise<Result<void, Failure[]>> => {
}
};

const optionsResult = getOptions();
const optionsResult = await getOptions();
if (!optionsResult.ok) {
// if we can't figure out our options, we can't figure out what logger
// we should use here, so just use the default logger.
const logger = new DefaultLogger(packageDir ?? process.cwd());
const console = new Console(process.stderr, process.stderr);
const logger = new DefaultLogger(packageDir ?? process.cwd(), console);
logger.log(optionsResult.error);
process.exit(1);
}
Expand Down
51 changes: 51 additions & 0 deletions src/logging/combination-logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* @license
* Copyright 2023 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import {Event} from '../event.js';
import {Console, Logger} from './logger.js';

// To prevent using the global console accidentally, we shadow it with
// undefined
const console = undefined;
function markAsUsed(_: unknown) {}
markAsUsed(console);

/**
* A {@link Logger} that logs to multiple loggers.
*/
export class CombinationLogger implements Logger {
readonly console: Console;
readonly #loggers: readonly Logger[];

constructor(loggers: readonly Logger[], console: Console) {
this.console = console;
this.#loggers = loggers;
}

log(event: Event): void {
for (const logger of this.#loggers) {
logger.log(event);
}
}
printMetrics(): void {
for (const logger of this.#loggers) {
logger.printMetrics?.();
}
}
getWatchLogger?(): Logger {
const watchLoggers = this.#loggers.map(
(logger) => logger.getWatchLogger?.() ?? logger,
);
return new CombinationLogger(watchLoggers, this.console);
}

[Symbol.dispose](): void {
for (const logger of this.#loggers) {
logger[Symbol.dispose]?.();
}
this.console[Symbol.dispose]();
}
}
47 changes: 47 additions & 0 deletions src/logging/debug-logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* @license
* Copyright 2023 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import {DefaultLogger} from './default-logger.js';
import {Event} from '../event.js';
import {inspect} from 'node:util';

// To prevent using the global console accidentally, we shadow it with
// undefined
const console = undefined;
function markAsUsed(_: unknown) {}
markAsUsed(console);

/**
* A {@link Logger} for logging debug information, mainly in tests.
*/
export class DebugLogger extends DefaultLogger {
override log(event: Event) {
switch (event.type) {
case 'info':
this.console.log(`<info> ${event.detail}`);
break;
case 'failure':
this.console.log(`<failure> ${event.reason}`);
break;
case 'output':
// too verbose, log nothing
return;
case 'success':
this.console.log(`<success> ${event.reason}`);
break;
default: {
const never: never = event;
throw new Error(`Unknown event type: ${inspect(never)}`);
}
}
super.log(event);
}

[Symbol.dispose](): void {
super[Symbol.dispose]();
this.console[Symbol.dispose]();
}
}
Loading

0 comments on commit ab17457

Please sign in to comment.