diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3c9928c2ad9..964f6da9df9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -216,6 +216,16 @@ To run tests against a specific folder in VSCode, do any one of: $Env:TEST_DIR = "../core/src/test/foo"; npm run test ``` +#### Run jscpd ("Copy-Paste Detection") + +If the "Copy-Paste Detection" CI job fails, you will find it useful to check things locally. To +check a specific file: + + npx jscpd --config .github/workflows/jscpd.json --pattern packages/…/src/foo.ts + +See the [jscpd cli documentation](https://github.com/kucherenko/jscpd/tree/master/apps/jscpd) for +more options. + ### Coverage report You can find the coverage report at `./coverage/amazonq/lcov-report/index.html` and `./coverage/toolkit/lcov-report/index.html` after running the tests. Tests ran from the workspace launch config won't generate a coverage report automatically because it can break file watching. diff --git a/packages/core/src/shared/logger/logger.ts b/packages/core/src/shared/logger/logger.ts index 3c602c2db35..89e7dc17796 100644 --- a/packages/core/src/shared/logger/logger.ts +++ b/packages/core/src/shared/logger/logger.ts @@ -4,7 +4,8 @@ */ import * as vscode from 'vscode' -import { LogTopic, type ToolkitLogger } from './toolkitLogger' + +export type LogTopic = 'crashReport' | 'notifications' | 'test' | 'unknown' const toolkitLoggers: { main: Logger | undefined @@ -33,6 +34,11 @@ export interface Logger { export abstract class BaseLogger implements Logger { logFile?: vscode.Uri + topic: LogTopic = 'unknown' + + setTopic(topic: LogTopic = 'unknown') { + this.topic = topic + } debug(message: string | Error, ...meta: any[]): number { return this.sendToLog('debug', message, ...meta) @@ -125,36 +131,20 @@ export function getLogger(topic?: LogTopic): Logger { if (!logger) { return new ConsoleLogger() } - /** - * need this check so that setTopic can be recognized - * using instanceof would lead to dependency loop error - */ - if (isToolkitLogger(logger)) { - logger.setTopic(topic) - } + ;(logger as BaseLogger).setTopic?.(topic) return logger } -/** - * check if the logger is of type `ToolkitLogger`. This avoids Circular Dependencies, but `instanceof ToolkitLogger` is preferred. - * @param logger - * @returns bool, true if is `ToolkitLogger` - */ -function isToolkitLogger(logger: Logger): logger is ToolkitLogger { - return 'setTopic' in logger && typeof logger.setTopic === 'function' -} - export function getDebugConsoleLogger(topic?: LogTopic): Logger { const logger = toolkitLoggers['debugConsole'] if (!logger) { return new ConsoleLogger() } - if (isToolkitLogger(logger)) { - logger.setTopic(topic) - } + ;(logger as BaseLogger).setTopic?.(topic) return logger } +// jscpd:ignore-start export class NullLogger extends BaseLogger { public setLogLevel(logLevel: LogLevel) {} public logLevelEnabled(logLevel: LogLevel): boolean { @@ -169,6 +159,9 @@ export class NullLogger extends BaseLogger { message: string | Error, ...meta: any[] ): number { + void logLevel + void message + void meta return 0 } } @@ -190,10 +183,7 @@ export class ConsoleLogger extends BaseLogger { message: string | Error, ...meta: any[] ): number { - /** - * This is here because we pipe verbose to debug currentlly - * TODO: remove in the next stage - */ + // TODO: we alias "verbose" to "debug" currently. Will be revisited: IDE-14839 if (logLevel === 'verbose') { // eslint-disable-next-line aws-toolkits/no-console-log console.debug(message, ...meta) @@ -204,10 +194,12 @@ export class ConsoleLogger extends BaseLogger { return 0 } } +// jscpd:ignore-end export function getNullLogger(type?: 'debugConsole' | 'main'): Logger { return new NullLogger() } + /** * Sets (or clears) the logger that is accessible to code. * The Extension is expected to call this only once per log type. diff --git a/packages/core/src/shared/logger/toolkitLogger.ts b/packages/core/src/shared/logger/toolkitLogger.ts index 9a3153ae9a1..7f655c7e4c2 100644 --- a/packages/core/src/shared/logger/toolkitLogger.ts +++ b/packages/core/src/shared/logger/toolkitLogger.ts @@ -14,9 +14,6 @@ import { SharedFileTransport } from './sharedFileTransport' import { ConsoleLogTransport } from './consoleLogTransport' import { isWeb } from '../extensionGlobals' -/* define log topics */ -export type LogTopic = 'unknown' | 'test' | 'crashReport' | 'notifications' - class ErrorLog { constructor( public topic: string, @@ -30,7 +27,6 @@ const logmapSize: number = 1000 export class ToolkitLogger extends BaseLogger implements vscode.Disposable { private readonly logger: winston.Logger /* topic is used for header in log messages, default is 'Unknown' */ - private topic: LogTopic = 'unknown' private disposed: boolean = false private idCounter: number = 0 private logMap: { [logID: number]: { [filePath: string]: string } } = {} @@ -115,16 +111,10 @@ export class ToolkitLogger extends BaseLogger implements vscode.Disposable { }) } - public setTopic(topic: LogTopic = 'unknown') { - this.topic = topic - } - /* Format the message with topic header */ private addTopicToMessage(message: string | Error): string | ErrorLog { if (typeof message === 'string') { - /*We shouldn't print unknow before current logging calls are migrated - * TODO: remove this once migration of current calls is completed - */ + // TODO: remove this after all calls are migrated and topic is a required param. if (this.topic === 'unknown') { return message } diff --git a/packages/core/src/test/shared/logger/toolkitLogger.test.ts b/packages/core/src/test/shared/logger/toolkitLogger.test.ts index 6f1ac17ee4f..b09cd5a46e7 100644 --- a/packages/core/src/test/shared/logger/toolkitLogger.test.ts +++ b/packages/core/src/test/shared/logger/toolkitLogger.test.ts @@ -239,32 +239,26 @@ describe('ToolkitLogger', function () { assert.ok(!(await waitForMessage).includes(nonLoggedVerboseEntry), 'unexpected message in log') }) - it('logs append topic header in message', async function () { - const testMessage = 'This is a test message' - const testMessageWithHeader = 'test: This is a test message' - + it('prepends topic to message', async function () { testLogger = new ToolkitLogger('info') testLogger.logToOutputChannel(outputChannel, false) testLogger.setTopic('test') testLogger.setLogLevel('verbose') - testLogger.verbose(testMessage) - - const waitForMessage = waitForLoggedTextByContents(testMessageWithHeader) - assert.ok((await waitForMessage).includes(testMessageWithHeader), 'Expected header added') - }) - it('logs append topic header in errors', async function () { - const testError = new ToolkitError('root error', { code: 'something went wrong' }) - const testErrorWithHeader = "topic: 'test'" + const testMessageWithHeader = 'test: This is a test message' + testLogger.verbose('This is a test message') + assert.ok( + (await waitForLoggedTextByContents(testMessageWithHeader)).includes(testMessageWithHeader), + 'Expected header added' + ) - testLogger = new ToolkitLogger('info') - testLogger.logToOutputChannel(outputChannel, false) - testLogger.setTopic('test') - testLogger.setLogLevel('verbose') - testLogger.verbose(testError) + const msg = "topic: 'test'" + testLogger.verbose(new ToolkitError('root error', { code: 'something went wrong' })) + assert.ok((await waitForLoggedTextByContents(msg)).includes(msg), 'Expected header added') - const waitForMessage = waitForLoggedTextByContents(testErrorWithHeader) - assert.ok((await waitForMessage).includes(testErrorWithHeader), 'Expected header added') + const msg2 = "topic: 'test'" + testLogger.verbose(new ToolkitError('root error', { code: 'something went wrong' })) + assert.ok((await waitForLoggedTextByContents(msg2)).includes(msg2), 'Expected header added') }) it('unknown topic header ignored in message', async function () { @@ -282,7 +276,7 @@ describe('ToolkitLogger', function () { assert.ok(!(await waitForMessage).includes(unknowntestMessage), 'unexpected header in log') }) - it('switch topic within same logger', async function () { + it('switch topic on same logger', async function () { const testMessage = 'This is a test message' const testMessageWithHeader = 'test: This is a test message'