Skip to content

Commit 609faba

Browse files
authored
chore: chalk breaking logging tests (#32393)
### Reason for this change logging tests would break when running `npx jest` but not when running `yarn test` as npx jest was not pulling out the ansi codes which were failing the string comparisons. ### Description of changes Removed all the ansi codes from stdout in logging and log-monitor tests ### Description of how you validated changes ran `npx jest` and `yarn test` as well as rebuilt the package ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 3fe229d commit 609faba

File tree

2 files changed

+45
-30
lines changed

2 files changed

+45
-30
lines changed

packages/aws-cdk/test/logging.test.ts renamed to packages/aws-cdk/test/api/logs/cli-logging.test.ts

+29-23
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
1-
import { LogLevel, log, setLogLevel, setCI, data, print, error, warning, success, debug, trace, prefix, withCorkedLogging } from '../lib/logging';
1+
import { LogLevel, log, setLogLevel, setCI, data, print, error, warning, success, debug, trace, prefix, withCorkedLogging } from '../../../lib/logging';
22

33
describe('logging', () => {
44
// Mock streams to capture output
55
let mockStdout: jest.Mock;
66
let mockStderr: jest.Mock;
77

8+
// Helper function to strip ANSI codes
9+
const stripAnsi = (str: string): string => {
10+
const ansiRegex = /\u001b\[[0-9;]*[a-zA-Z]/g;
11+
return str.replace(ansiRegex, '');
12+
};
13+
814
beforeEach(() => {
915
// Reset log level before each test
1016
setLogLevel(LogLevel.INFO);
@@ -14,44 +20,45 @@ describe('logging', () => {
1420
mockStdout = jest.fn();
1521
mockStderr = jest.fn();
1622

17-
// Mock the write methods directly
23+
// Mock the write methods directly and strip ANSI codes
1824
jest.spyOn(process.stdout, 'write').mockImplementation((chunk: any) => {
19-
mockStdout(chunk.toString());
25+
mockStdout(stripAnsi(chunk.toString()));
2026
return true;
2127
});
2228

2329
jest.spyOn(process.stderr, 'write').mockImplementation((chunk: any) => {
24-
mockStderr(chunk.toString());
30+
mockStderr(stripAnsi(chunk.toString()));
2531
return true;
2632
});
2733
});
2834

2935
afterEach(() => {
3036
jest.restoreAllMocks();
3137
});
38+
3239
describe('stream selection', () => {
3340
test('data() always writes to stdout', () => {
3441
data('test message');
35-
expect(mockStdout).toHaveBeenCalledWith(expect.stringContaining('test message\n'));
42+
expect(mockStdout).toHaveBeenCalledWith('test message\n');
3643
expect(mockStderr).not.toHaveBeenCalled();
3744
});
3845

3946
test('error() always writes to stderr', () => {
4047
error('test error');
41-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('test error\n'));
48+
expect(mockStderr).toHaveBeenCalledWith('test error\n');
4249
expect(mockStdout).not.toHaveBeenCalled();
4350
});
4451

4552
test('print() writes to stderr by default', () => {
4653
print('test print');
47-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('test print\n'));
54+
expect(mockStderr).toHaveBeenCalledWith('test print\n');
4855
expect(mockStdout).not.toHaveBeenCalled();
4956
});
5057

5158
test('print() writes to stdout in CI mode', () => {
5259
setCI(true);
5360
print('test print');
54-
expect(mockStdout).toHaveBeenCalledWith(expect.stringContaining('test print\n'));
61+
expect(mockStdout).toHaveBeenCalledWith('test print\n');
5562
expect(mockStderr).not.toHaveBeenCalled();
5663
});
5764
});
@@ -62,9 +69,9 @@ describe('logging', () => {
6269
error('error message');
6370
warning('warning message');
6471
print('print message');
65-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('error message\n'));
66-
expect(mockStderr).not.toHaveBeenCalledWith(expect.stringContaining('warning message\n'));
67-
expect(mockStderr).not.toHaveBeenCalledWith(expect.stringContaining('print message\n'));
72+
expect(mockStderr).toHaveBeenCalledWith('error message\n');
73+
expect(mockStderr).not.toHaveBeenCalledWith('warning message\n');
74+
expect(mockStderr).not.toHaveBeenCalledWith('print message\n');
6875
});
6976

7077
test('debug messages only show at debug level', () => {
@@ -74,7 +81,7 @@ describe('logging', () => {
7481

7582
setLogLevel(LogLevel.DEBUG);
7683
debug('debug message');
77-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('debug message\n'));
84+
expect(mockStderr).toHaveBeenCalledWith('debug message\n');
7885
});
7986

8087
test('trace messages only show at trace level', () => {
@@ -84,26 +91,25 @@ describe('logging', () => {
8491

8592
setLogLevel(LogLevel.TRACE);
8693
trace('trace message');
87-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('trace message\n'));
94+
expect(mockStderr).toHaveBeenCalledWith('trace message\n');
8895
});
8996
});
9097

9198
describe('message formatting', () => {
9299
test('formats messages with multiple arguments', () => {
93100
print('Value: %d, String: %s', 42, 'test');
94-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('Value: 42, String: test\n'));
101+
expect(mockStderr).toHaveBeenCalledWith('Value: 42, String: test\n');
95102
});
96103

97104
test('handles prefix correctly', () => {
98105
const prefixedLog = prefix('PREFIX');
99106
prefixedLog('test message');
100-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('PREFIX test message\n'));
107+
expect(mockStderr).toHaveBeenCalledWith('PREFIX test message\n');
101108
});
102109

103110
test('handles custom styles', () => {
104111
success('success message');
105-
// Note: actual styling will depend on chalk, but we can verify the message is there
106-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('success message\n'));
112+
expect(mockStderr).toHaveBeenCalledWith('success message\n');
107113
});
108114
});
109115

@@ -115,8 +121,8 @@ describe('logging', () => {
115121
expect(mockStderr).not.toHaveBeenCalled();
116122
});
117123

118-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('message 1\n'));
119-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('message 2\n'));
124+
expect(mockStderr).toHaveBeenCalledWith('message 1\n');
125+
expect(mockStderr).toHaveBeenCalledWith('message 2\n');
120126
});
121127

122128
test('handles nested corking correctly', async () => {
@@ -130,9 +136,9 @@ describe('logging', () => {
130136
});
131137

132138
expect(mockStderr).toHaveBeenCalledTimes(3);
133-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('outer 1\n'));
134-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('inner\n'));
135-
expect(mockStderr).toHaveBeenCalledWith(expect.stringContaining('outer 2\n'));
139+
expect(mockStderr).toHaveBeenCalledWith('outer 1\n');
140+
expect(mockStderr).toHaveBeenCalledWith('inner\n');
141+
expect(mockStderr).toHaveBeenCalledWith('outer 2\n');
136142
});
137143
});
138144

@@ -145,7 +151,7 @@ describe('logging', () => {
145151
prefix: 'PREFIX',
146152
});
147153
expect(mockStderr).toHaveBeenCalledWith(
148-
expect.stringMatching(/PREFIX \[\d{2}:\d{2}:\d{2}\] test message\n/),
154+
expect.stringMatching(/^PREFIX \[\d{2}:\d{2}:\d{2}\] test message\n$/),
149155
);
150156
});
151157
});

packages/aws-cdk/test/api/logs/logs-monitor.test.ts

+16-7
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,25 @@
11
import { FilterLogEventsCommand, type FilteredLogEvent } from '@aws-sdk/client-cloudwatch-logs';
2-
import { blue, yellow } from 'chalk';
32
import { CloudWatchLogEventMonitor } from '../../../lib/api/logs/logs-monitor';
43
import { sleep } from '../../util';
54
import { MockSdk, mockCloudWatchClient } from '../../util/mock-sdk';
65

6+
// Helper function to strip ANSI codes
7+
const stripAnsi = (str: string): string => {
8+
const ansiRegex = /\u001b\[[0-9;]*[a-zA-Z]/g;
9+
return str.replace(ansiRegex, '');
10+
};
11+
712
let sdk: MockSdk;
813
let stderrMock: jest.SpyInstance;
914
let monitor: CloudWatchLogEventMonitor;
1015
beforeEach(() => {
1116
monitor = new CloudWatchLogEventMonitor(new Date(T100));
12-
stderrMock = jest.spyOn(process.stderr, 'write').mockImplementation(() => {
13-
return true;
17+
stderrMock = jest.spyOn(process.stderr, 'write').mockImplementation((chunk: any) => {
18+
// Strip ANSI codes when capturing output
19+
if (typeof chunk === 'string') {
20+
return stripAnsi(chunk) as unknown as boolean;
21+
}
22+
return stripAnsi(chunk.toString()) as unknown as boolean;
1423
});
1524
sdk = new MockSdk();
1625
});
@@ -44,7 +53,7 @@ test('process events', async () => {
4453
// THEN
4554
const expectedLocaleTimeString = eventDate.toLocaleTimeString();
4655
expect(stderrMock).toHaveBeenCalledTimes(1);
47-
expect(stderrMock.mock.calls[0][0]).toContain(`[${blue('loggroup')}] ${yellow(expectedLocaleTimeString)} message`);
56+
expect(stripAnsi(stderrMock.mock.calls[0][0])).toContain(`[loggroup] ${expectedLocaleTimeString} message`);
4857
});
4958

5059
test('process truncated events', async () => {
@@ -76,9 +85,9 @@ test('process truncated events', async () => {
7685
// THEN
7786
const expectedLocaleTimeString = eventDate.toLocaleTimeString();
7887
expect(stderrMock).toHaveBeenCalledTimes(101);
79-
expect(stderrMock.mock.calls[0][0]).toContain(`[${blue('loggroup')}] ${yellow(expectedLocaleTimeString)} message`);
80-
expect(stderrMock.mock.calls[100][0]).toContain(
81-
`[${blue('loggroup')}] ${yellow(expectedLocaleTimeString)} >>> \`watch\` shows only the first 100 log messages - the rest have been truncated...`,
88+
expect(stripAnsi(stderrMock.mock.calls[0][0])).toContain(`[loggroup] ${expectedLocaleTimeString} message0`);
89+
expect(stripAnsi(stderrMock.mock.calls[100][0])).toContain(
90+
`[loggroup] ${expectedLocaleTimeString} >>> \`watch\` shows only the first 100 log messages - the rest have been truncated...`,
8291
);
8392
});
8493

0 commit comments

Comments
 (0)