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

chore: unconnected CliIoHost logger-only implementation #32503

Merged
merged 27 commits into from
Dec 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a978966
initial toolkit implementation
HBobertz Dec 12, 2024
953fb3d
fix linting problems
HBobertz Dec 12, 2024
66ef2f5
Merge branch 'main' into bobertzh/toolkit
HBobertz Dec 13, 2024
98888d4
address iohost feedback
HBobertz Dec 24, 2024
634b3eb
Merge branch 'main' into bobertzh/toolkit
HBobertz Dec 24, 2024
09b847f
linting fixes
HBobertz Dec 24, 2024
255339e
cli-io-host stream fixes
HBobertz Dec 24, 2024
6d09f68
address feedback
HBobertz Dec 26, 2024
eb24b3b
update cli-io-host with feedback
HBobertz Dec 26, 2024
e2ec9f6
address feedback
HBobertz Dec 26, 2024
9c4c7d6
Update packages/aws-cdk/test/toolkit/cli-io-host.test.ts
HBobertz Dec 26, 2024
f910e94
address feedback 2
HBobertz Dec 26, 2024
dec1bc1
fix test
HBobertz Dec 26, 2024
8b1f0fb
move comment to function body
HBobertz Dec 26, 2024
c12bb2f
add default
HBobertz Dec 26, 2024
fbedcd2
change params to properties
HBobertz Dec 27, 2024
172e312
refactor code
HBobertz Dec 27, 2024
1c04123
update cli io host comments
HBobertz Dec 27, 2024
34b497c
Merge branch 'main' into bobertzh/toolkit
HBobertz Dec 27, 2024
60c2e48
update code string
HBobertz Dec 27, 2024
dadef91
add fixes
HBobertz Dec 27, 2024
5deae51
remove tsdocs
HBobertz Dec 27, 2024
86b9734
remove word
HBobertz Dec 27, 2024
542a56d
move space
HBobertz Dec 27, 2024
b99d7b4
Apply suggestions from code review
kaizencc Dec 27, 2024
bc0ccbd
Merge branch 'main' into bobertzh/toolkit
HBobertz Dec 27, 2024
dc56d3e
Merge branch 'main' into bobertzh/toolkit
mergify[bot] Dec 27, 2024
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
204 changes: 204 additions & 0 deletions packages/aws-cdk/test/toolkit/toolkit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
import * as chalk from 'chalk';
import { _private } from '../../toolkit/toolkit';

const CliIoHost = _private.CliIoHost;
const IoMessageLevel = _private.IoMessageLevel;
const IoAction = _private.IoAction;

describe('CliIoHost', () => {
let mockStdout: jest.Mock;
let mockStderr: jest.Mock;

beforeEach(() => {
mockStdout = jest.fn();
mockStderr = jest.fn();

// MOck the write methods of STD out and STD err
jest.spyOn(process.stdout, 'write').mockImplementation((str: any, encoding?: any, cb?: any) => {
mockStdout(str.toString());
// Handle callback
const callback = typeof encoding === 'function' ? encoding : cb;
if (callback) callback();
return true;
});

jest.spyOn(process.stderr, 'write').mockImplementation((str: any, encoding?: any, cb?: any) => {
mockStderr(str.toString());
// Handle callback
const callback = typeof encoding === 'function' ? encoding : cb;
if (callback) callback();
return true;
});
});

afterEach(() => {
jest.restoreAllMocks();
});

describe('stream selection', () => {
test('writes to stdout by default', async () => {
const host = new CliIoHost({ useTTY: true });
await host.notify({
time: new Date(),
level: IoMessageLevel.INFO,
action: IoAction.SYNTH,
code: 'TEST',
message: 'test message',
});

expect(mockStdout).toHaveBeenCalledWith(chalk.white('test message') + '\n');
expect(mockStderr).not.toHaveBeenCalled();
});

test('writes to stderr for error level with red color', async () => {
const host = new CliIoHost({ useTTY: true });
await host.notify({
time: new Date(),
level: IoMessageLevel.ERROR,
action: IoAction.SYNTH,
code: 'TEST',
message: 'error message',
});

expect(mockStderr).toHaveBeenCalledWith(chalk.red('error message') + '\n');
expect(mockStdout).not.toHaveBeenCalled();
});
});

describe('TTY formatting', () => {
test('accepts custom chalk styles', async () => {
const host = new CliIoHost({ useTTY: true });
await host.notify({
time: new Date(),
level: IoMessageLevel.INFO,
action: IoAction.SYNTH,
code: 'TEST',
message: 'green message',
style: chalk.green,
});

expect(mockStdout).toHaveBeenCalledWith(chalk.green('green message') + '\n');
});

test('applies custom style in TTY mode', async () => {
const host = new CliIoHost({ useTTY: true });
const customStyle = (str: string) => `\x1b[35m${str}\x1b[0m`; // Custom purple color

await host.notify({
time: new Date(),
level: IoMessageLevel.INFO,
action: IoAction.SYNTH,
code: 'TEST',
message: 'styled message',
style: customStyle,
});

expect(mockStdout).toHaveBeenCalledWith(customStyle('styled message') + '\n');
});

test('applies default style by message level in TTY mode', async () => {
const host = new CliIoHost({ useTTY: true });
await host.notify({
time: new Date(),
level: IoMessageLevel.WARN,
action: IoAction.SYNTH,
code: 'TEST',
message: 'warning message',
});

expect(mockStdout).toHaveBeenCalledWith(chalk.yellow('warning message') + '\n');
});

test('does not apply styles in non-TTY mode', async () => {
const host = new CliIoHost({ useTTY: false });
await host.notify({
time: new Date(),
level: IoMessageLevel.INFO,
action: IoAction.SYNTH,
code: 'TEST',
message: 'unstyled message',
});

expect(mockStdout).toHaveBeenCalledWith('unstyled message\n');
});

test('does not apply styles in non-TTY mode with style provided', async () => {
const host = new CliIoHost({ useTTY: false });
await host.notify({
time: new Date(),
level: IoMessageLevel.INFO,
action: IoAction.SYNTH,
code: 'TEST',
message: 'unstyled message',
style: chalk.green,
});

expect(mockStdout).toHaveBeenCalledWith('unstyled message\n');
});
});

describe('timestamp handling', () => {
test('includes timestamp for DEBUG level with gray color', async () => {
const host = new CliIoHost({ useTTY: true });
const testDate = new Date('2024-01-01T12:34:56');

await host.notify({
time: testDate,
level: IoMessageLevel.DEBUG,
action: IoAction.SYNTH,
code: 'TEST',
message: 'debug message',
});

expect(mockStdout).toHaveBeenCalledWith(`[12:34:56] ${chalk.gray('debug message')}\n`);
});

test('includes timestamp for TRACE level with gray color', async () => {
const host = new CliIoHost({ useTTY: true });
const testDate = new Date('2024-01-01T12:34:56');

await host.notify({
time: testDate,
level: IoMessageLevel.TRACE,
action: IoAction.SYNTH,
code: 'TEST',
message: 'trace message',
});

expect(mockStdout).toHaveBeenCalledWith(`[12:34:56] ${chalk.gray('trace message')}\n`);
});

test('excludes timestamp for other levels but includes color', async () => {
const host = new CliIoHost({ useTTY: true });
const testDate = new Date('2024-01-01T12:34:56');

await host.notify({
time: testDate,
level: IoMessageLevel.INFO,
action: IoAction.SYNTH,
code: 'TEST',
message: 'info message',
});

expect(mockStdout).toHaveBeenCalledWith(chalk.white('info message') + '\n');
});
});

// describe('error handling', () => {
// test('rejects on write error', async () => {
// jest.spyOn(process.stdout, 'write').mockImplementation((_: any, callback: any) => {
// callback(new Error('Write failed'));
// return true;
// });

// const host = new CliIoHost({ useTTY: true });
// await expect(host.notify({
// time: new Date(),
// level: IoMessageLevel.INFO,
// action: IoAction.SYNTH,
// code: 'TEST',
// message: 'test message',
// })).rejects.toThrow('Write failed');
// });
// });
});
106 changes: 106 additions & 0 deletions packages/aws-cdk/toolkit/toolkit.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably want to call this file after its contents.

This stuff still needs to be in lib.

Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import * as chalk from 'chalk';

/**
* Basic message structure for toolkit notifications.
* Messages are emitted by the toolkit and handled by the IoHost.
*/
interface IoMessage {
time: Date;
level: IoMessageLevel;
action: IoAction;
code: string;
message: string;
// Specify Chalk style for stdout/stderr, if TTY is enabled
style?: ((str: string) => string);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good thought. Is this currently used anywhere?

Beyond that, that - there's an interesting question for you: We currently use chalk inline in a number of places. How are we going to deal with that?

Might be easier to always chalk and use a global setting to deal with color vs no color.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not currently used anywhere, but I thought this would be a nice thing to add for customers when we create the IoHost interface

Copy link
Contributor Author

@HBobertz HBobertz Dec 24, 2024

Choose a reason for hiding this comment

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

I did some more recon on this and this is a very real problem I'm gonna need to address in a couple spots. This is enough though that I want to break that out into a separate pr, so I'll handle that in a follow up

Copy link
Contributor

Choose a reason for hiding this comment

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

can you explain the problem here?

}

enum IoMessageLevel {
ERROR = 'error',
WARN = 'warn',
INFO = 'info',
DEBUG = 'debug',
TRACE = 'trace',
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make this a type instead of an enum.


enum IoAction {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make this a type instead of an enum.

SYNTH = 'synth',
LIST = 'list',
DEPLOY = 'deploy',
DESTROY = 'destroy',
}

/**
* A simple IO host for the CLI that writes messages to the console.
*/
class CliIoHost {
private readonly useTTY: boolean;

constructor(options: { useTTY?: boolean } = {}) {
this.useTTY = options.useTTY ?? process.stdout.isTTY ?? false;
}

/**
* Notifies the host of a message.
* The caller waits until the notification completes.
*/
async notify(msg: IoMessage): Promise<void> {
const output = this.formatMessage(msg);

const stream = msg.level === 'error' ? process.stderr : process.stdout;
Copy link
Contributor

Choose a reason for hiding this comment

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

stream selection doesn't match what we are currently doing in this regard.


return new Promise((resolve, reject) => {
stream.write(output + '\n', (err) => {
if (err) {
reject(err);
} else {
resolve();
}
});
});
}

/**
* Formats a message for console output with optional color support
*/
private formatMessage(msg: IoMessage): string {
// apply provided style or a default style if we're in TTY mode
let output = this.useTTY
? (msg.style?.(msg.message) ?? styleMap[msg.level](msg.message))
: msg.message;

// prepend timestamp if IoMessageLevel is DEBUG or TRACE
return (msg.level === IoMessageLevel.DEBUG || msg.level === IoMessageLevel.TRACE)
? `[${this.formatTime(msg.time)}] ${output}`
: output;
}

/**
* Formats date to HH:MM:SS
*/
private formatTime(d: Date): string {
const pad = (n: number): string => n.toString().padStart(2, '0');
return `${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}`;
}

}

const styleMap: Record<IoMessageLevel, (str: string) => string> = {
[IoMessageLevel.ERROR]: chalk.red,
[IoMessageLevel.WARN]: chalk.yellow,
[IoMessageLevel.INFO]: chalk.white,
[IoMessageLevel.DEBUG]: chalk.gray,
[IoMessageLevel.TRACE]: chalk.gray,
};

/**
* @internal
* Used by the toolkit unit tests.
* These APIs are not part of the public interface and will change without notice.
* Do Not Use.
*
*/
export const _private = {
CliIoHost,
IoMessageLevel,
IoAction,
} as const;
Loading