Skip to content

Commit

Permalink
fix: log spacing depending on the FORMAT and COLORS options (#4631)
Browse files Browse the repository at this point in the history
* fix: Bad log spacing depending on the FORMAT and COLORS options used

fixes: #4630

inserted a space between the timestamp and the message when logging timestamped messages.

* fix: Bad log spacing depending on the FORMAT and COLORS options used

fixes: #4630

removed padding of an unnecessary space (at the start or end of the log string, depending on whether colors are enabled).

* remove padLeft, update tests

* update logger-commons tests

---------

Co-authored-by: Marc Bernard <marc@marcbernardtools.com>
  • Loading branch information
somethingSTRANGE and mbtools committed Jun 1, 2024
1 parent e5624e1 commit cf1b46c
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 28 deletions.
6 changes: 6 additions & 0 deletions .changeset/dry-shoes-report.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@verdaccio/logger-commons': patch
'@verdaccio/logger-prettify': patch
---

fix: log spacing depending on the FORMAT and COLORS options
8 changes: 4 additions & 4 deletions packages/logger/logger-commons/test/logger.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('logger test', () => {
logger.trace(`this should not be logged`);
logger.error(`this should logged`);
const content = await readLogFile(file);
expect(content).toBe('info --- testing test \nerror--- this should logged \n');
expect(content).toBe('info --- testing test\nerror--- this should logged\n');
});

test('should include all logging level', async () => {
Expand All @@ -51,7 +51,7 @@ describe('logger test', () => {
logger.error(`this should logged`);
const content = await readLogFile(file);
expect(content).toBe(
'info --- testing test \ndebug--- this should not be logged \ntrace--- this should not be logged \nerror--- this should logged \n'
'info --- testing test\ndebug--- this should not be logged\ntrace--- this should not be logged\nerror--- this should logged\n'
);
});
});
Expand Down Expand Up @@ -101,7 +101,7 @@ describe('logger test', () => {
`publishing or updating a new version for @{packageName}`
);
const content = await readLogFile(file);
expect(content).toEqual('info --- publishing or updating a new version for test \n');
expect(content).toEqual('info --- publishing or updating a new version for test\n');
});

test('should log into a file with pretty-timestamped', async () => {
Expand All @@ -122,7 +122,7 @@ describe('logger test', () => {
);
const content = await readLogFile(file);
// TODO: we might want mock time for testing
expect(content).toMatch('info --- publishing or updating a new version for test \n');
expect(content).toMatch('info --- publishing or updating a new version for test\n');
});
});
});
6 changes: 3 additions & 3 deletions packages/logger/logger-prettify/src/formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { inspect } from 'util';

import { LevelCode, calculateLevel, levelsColors, subSystemLevels } from './levels';
import { PrettyOptionsExtended } from './types';
import { formatLoggingDate, isObject, padLeft, padRight } from './utils';
import { formatLoggingDate, isObject, padRight } from './utils';

let LEVEL_VALUE_MAX = 0;
for (const l in levelsColors) {
Expand Down Expand Up @@ -68,11 +68,11 @@ function getMessage(debugLevel, msg, sub, templateObjects, hasColors: boolean) {
`${subSystemType} ${finalMessage}`
)}`;

return padLeft(logString);
return logString;
}
const logString = `${padRight(debugLevel, LEVEL_VALUE_MAX)}${subSystemType} ${finalMessage}`;

return padRight(logString);
return logString;
}

export function printMessage(
Expand Down
6 changes: 1 addition & 5 deletions packages/logger/logger-prettify/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,12 @@ export function isObject(obj: unknown): boolean {
return _.isObject(obj) && _.isNull(obj) === false && _.isArray(obj) === false;
}

export function padLeft(message: string) {
return message.padStart(message.length + CUSTOM_PAD_LENGTH, ' ');
}

export function padRight(message: string, max = message.length + CUSTOM_PAD_LENGTH) {
return message.padEnd(max, ' ');
}

export function formatLoggingDate(time: number, message: string): string {
const timeFormatted = dayjs(time).format(FORMAT_DATE);

return `[${timeFormatted}]${message}`;
return `[${timeFormatted}] ${message}`;
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`formatter printMessage should display a bytes request 1`] = `"fatal<-- 200, user: null(127.0.0.1), req: 'GET /verdaccio', bytes: 0/150186 "`;
exports[`formatter printMessage should display a bytes request 1`] = `"fatal<-- 200, user: null(127.0.0.1), req: 'GET /verdaccio', bytes: 0/150186"`;

exports[`formatter printMessage should display a resource request 1`] = `"info <-- 127.0.0.1 requested 'GET /verdaccio' "`;
exports[`formatter printMessage should display a resource request 1`] = `"info <-- 127.0.0.1 requested 'GET /verdaccio'"`;

exports[`formatter printMessage should display a streaming request 1`] = `"fatal--> 304, req: 'GET https://registry.npmjs.org/verdaccio' (streaming) "`;
exports[`formatter printMessage should display a streaming request 1`] = `"fatal--> 304, req: 'GET https://registry.npmjs.org/verdaccio' (streaming)"`;

exports[`formatter printMessage should display an error request 1`] = `"fatal--> ERR, req: 'GET https://registry.fake.org/aaa', error: getaddrinfo ENOTFOUND registry.fake.org "`;
exports[`formatter printMessage should display an error request 1`] = `"fatal--> ERR, req: 'GET https://registry.fake.org/aaa', error: getaddrinfo ENOTFOUND registry.fake.org"`;

exports[`formatter printMessage should display an fatal request 1`] = `"fatal--> ERR, req: 'GET https://registry.fake.org/aaa', error: fatal error "`;
exports[`formatter printMessage should display an fatal request 1`] = `"fatal--> ERR, req: 'GET https://registry.fake.org/aaa', error: fatal error"`;

exports[`formatter printMessage should display config file 1`] = `"warn --- config file - /Users/user/.config/verdaccio/config/config.yaml "`;
exports[`formatter printMessage should display config file 1`] = `"warn --- config file - /Users/user/.config/verdaccio/config/config.yaml"`;

exports[`formatter printMessage should display custom log message 1`] = `"fatal--- custom - foo - undefined "`;
exports[`formatter printMessage should display custom log message 1`] = `"fatal--- custom - foo - undefined"`;

exports[`formatter printMessage should display trace level 1`] = `"trace--- [trace] - foo "`;
exports[`formatter printMessage should display trace level 1`] = `"trace--- [trace] - foo"`;

exports[`formatter printMessage should display trace level with pretty stamp 1`] = `"[formatted-date]trace--- [trace] - foo "`;
exports[`formatter printMessage should display trace level with pretty stamp 1`] = `"[formatted-date] trace--- [trace] - foo"`;

exports[`formatter printMessage should display version and http address 1`] = `"warn --- http address - http://localhost:4873/ - verdaccio/5.0.0 "`;
exports[`formatter printMessage should display version and http address 1`] = `"warn --- http address - http://localhost:4873/ - verdaccio/5.0.0"`;
2 changes: 1 addition & 1 deletion packages/logger/logger-prettify/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('prettyFactory', () => {
objectMode: true,
write(chunk, enc, cb) {
const formatted = pretty(JSON.parse(chunk));
expect(formatted).toBe('info --- test message ');
expect(formatted).toBe('info --- test message');
cb();
done();
},
Expand Down
7 changes: 2 additions & 5 deletions packages/logger/logger-prettify/test/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { formatLoggingDate, padLeft, padRight } from '../src/utils';
import { formatLoggingDate, padRight } from '../src/utils';

describe('formatLoggingDate', () => {
test('basic', () => {
expect(formatLoggingDate(1585411248203, ' message')).toEqual('[2020-03-28 16:00:48] message');
expect(formatLoggingDate(1585411248203, 'message')).toEqual('[2020-03-28 16:00:48] message');
});
});

Expand All @@ -14,7 +14,4 @@ describe('pad', () => {
test('pad right 2', () => {
expect(padRight('message right')).toEqual('message right ');
});
test('pad left', () => {
expect(padLeft('message left')).toEqual(' message left');
});
});

0 comments on commit cf1b46c

Please sign in to comment.