Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: oclif-friendly error logging #1080

Merged
merged 4 commits into from
Nov 21, 2024
Merged
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
9 changes: 4 additions & 5 deletions __tests__/commands/docs/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import fs from 'node:fs';
import path from 'node:path';

import { runCommand as oclifRunCommand } from '@oclif/test';
import chalk from 'chalk';
import frontMatter from 'gray-matter';
import nock from 'nock';
Expand All @@ -16,7 +15,7 @@ import { getAPIV1Mock, getAPIV1MockWithVersionHeader } from '../../helpers/get-a
import { after, before } from '../../helpers/get-gha-setup.js';
import hashFileContents from '../../helpers/hash-file-contents.js';
import { after as afterGHAEnv, before as beforeGHAEnv } from '../../helpers/setup-gha-env.js';
import { runCommand } from '../../helpers/setup-oclif-config.js';
import { runCommand, runCommandWithHooks } from '../../helpers/setup-oclif-config.js';

const fixturesBaseDir = '__fixtures__/docs';
const fullFixturesDir = `${__dirname}./../../${fixturesBaseDir}`;
Expand Down Expand Up @@ -722,9 +721,9 @@ describe('rdme docs', () => {

describe('rdme guides', () => {
it('should error if no path provided', async () => {
return expect((await oclifRunCommand(['guides', '--key', key, '--version', '1.0.0'])).error.message).toContain(
'Missing 1 required arg:\npath',
);
return expect(
(await runCommandWithHooks(['guides', '--key', key, '--version', '1.0.0'])).error.message,
).toContain('Missing 1 required arg:\npath');
});
});
});
5 changes: 2 additions & 3 deletions __tests__/commands/docs/prune.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { runCommand as oclifRunCommand } from '@oclif/test';
import nock from 'nock';
import prompts from 'prompts';
import { describe, beforeAll, afterAll, it, expect } from 'vitest';

import Command from '../../../src/commands/docs/prune.js';
import { getAPIV1Mock, getAPIV1MockWithVersionHeader } from '../../helpers/get-api-mock.js';
import { runCommand } from '../../helpers/setup-oclif-config.js';
import { runCommand, runCommandWithHooks } from '../../helpers/setup-oclif-config.js';

const fixturesBaseDir = '__fixtures__/docs';

Expand Down Expand Up @@ -166,7 +165,7 @@ describe('rdme docs:prune', () => {
describe('rdme guides:prune', () => {
it('should error if no folder provided', async () => {
return expect(
(await oclifRunCommand(['guides:prune', '--key', key, '--version', version])).error.message,
(await runCommandWithHooks(['guides:prune', '--key', key, '--version', version])).error.message,
).toContain('Missing 1 required arg:\nfolder');
});
});
Expand Down
5 changes: 2 additions & 3 deletions __tests__/commands/openapi/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@

import fs from 'node:fs';

import { runCommand as oclifRunCommand } from '@oclif/test';
import chalk from 'chalk';
import prompts from 'prompts';
import { describe, beforeAll, beforeEach, afterEach, it, expect, vi, type MockInstance } from 'vitest';

import Command from '../../../src/commands/openapi/validate.js';
import { after, before } from '../../helpers/get-gha-setup.js';
import { runCommand } from '../../helpers/setup-oclif-config.js';
import { runCommand, runCommandWithHooks } from '../../helpers/setup-oclif-config.js';

let consoleInfoSpy: MockInstance;

Expand Down Expand Up @@ -122,7 +121,7 @@ describe('rdme openapi:validate', () => {
it('should fail if user attempts to pass `--github` flag in CI environment', async () => {
return expect(
(
await oclifRunCommand([
await runCommandWithHooks([
'openapi:validate',
'__tests__/__fixtures__/petstore-simple-weird-version.json',
'--github',
Expand Down
27 changes: 18 additions & 9 deletions __tests__/helpers/setup-oclif-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import type { Command as OclifCommand } from '@oclif/core';
import path from 'node:path';

import { Config } from '@oclif/core';
import { captureOutput } from '@oclif/test';
import { captureOutput, runCommand as oclifRunCommand } from '@oclif/test';

const testNodeEnv = process.env.NODE_ENV;

/**
* Used for setting up the oclif configuration for simulating commands in tests.
Expand All @@ -28,13 +30,20 @@ export function runCommand<T extends typeof OclifCommand>(Command: T) {
const oclifConfig = await setupOclifConfig();
// @ts-expect-error this is the pattern recommended by the @oclif/test docs.
// Not sure how to get this working with type generics.
return captureOutput<string>(() => Command.run(args, oclifConfig), { testNodeEnv: 'rdme-test' }).then(
({ error, result }) => {
if (error) {
throw error;
}
return result;
},
);
return captureOutput<string>(() => Command.run(args, oclifConfig), { testNodeEnv }).then(({ error, result }) => {
if (error) {
throw error;
}
return result;
});
};
}

/**
* A lightweight wrapper around `@oclif/test`'s `runCommand`
* that loads our mock config and mock test env.
*/
export async function runCommandWithHooks(args?: string[]) {
const oclifConfig = await setupOclifConfig();
return oclifRunCommand(args, oclifConfig, { testNodeEnv });
}
15 changes: 0 additions & 15 deletions __tests__/setup.js

This file was deleted.

39 changes: 35 additions & 4 deletions src/lib/baseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { format } from 'node:util';

import * as core from '@actions/core';
import { Command as OclifCommand } from '@oclif/core';
import chalk from 'chalk';
import debugPkg from 'debug';

import { isGHA, isTest } from './isCI.js';
Expand Down Expand Up @@ -44,10 +45,40 @@ export default abstract class BaseCommand<T extends typeof OclifCommand> extends

abstract run(): Promise<string>;

protected async catch(err: Error & { exitCode?: number }): Promise<unknown> {
// add any custom logic to handle errors from the command
// or simply return the parent class error handling
return super.catch(err);
protected async catch(err: Error & { exitCode?: number }) {
if (isTest()) {
return super.catch(err);
}

let message = `Yikes, something went wrong! Please try again and if the problem persists, get in touch with our support team at ${chalk.underline(
'support@readme.io',
)}.`;

if (err.message) {
message = err.message;
}

/**
* If we're in a GitHub Actions environment, log errors with that formatting instead.
*
* @see {@link https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message}
* @see {@link https://github.com/actions/toolkit/tree/main/packages/core#annotations}
*/
if (isGHA()) {
return core.setFailed(message);
}

// If this is a soft error then we should output the result as a regular log but exit the CLI
// with an error status code.
if (err.name === 'SoftError') {
// eslint-disable-next-line no-console
console.log(err.message);
} else {
// eslint-disable-next-line no-console
console.error(chalk.red(`\n${message}\n`));
}

return process.exit(process.exitCode ?? err.exitCode ?? 1);
}

/**
Expand Down
15 changes: 14 additions & 1 deletion vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,27 @@ export default defineConfig({
coverage: {
exclude: [...coverageConfigDefaults.exclude, '**/dist-gha/**'],
},
env: {
/**
* The `chalk` and `colors` libraries have trouble with tests sometimes in test snapshots so
* we're disabling colorization here for all tests.
*
* @see {@link https://github.com/chalk/supports-color/issues/106}
*/
FORCE_COLOR: '0',
/**
* Sets our test `NODE_ENV` to a custom value in case of false positives if someone is using this
* tool in a testing environment.
*/
NODE_ENV: 'rdme-test',
},
exclude: [
'**/__fixtures__/**',
'**/dist-gha/**',
'**/helpers/**',
'**/__snapshots__/**',
...configDefaults.exclude,
],
globalSetup: ['./__tests__/setup.js'],
onConsoleLog(log: string, type: 'stderr' | 'stdout'): boolean | void {
// hides `rdme open` deprecation warning
return !(log.includes('`rdme open` is deprecated and will be removed in a future release') && type === 'stderr');
Expand Down
Loading