Skip to content

Commit

Permalink
Merge master into feature/cwltail
Browse files Browse the repository at this point in the history
  • Loading branch information
aws-toolkit-automation authored Nov 6, 2024
2 parents 66b2f46 + 5c37d1d commit b364573
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 55 deletions.
10 changes: 10 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
40 changes: 16 additions & 24 deletions packages/core/src/shared/logger/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -169,6 +159,9 @@ export class NullLogger extends BaseLogger {
message: string | Error,
...meta: any[]
): number {
void logLevel
void message
void meta
return 0
}
}
Expand All @@ -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)
Expand All @@ -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.
Expand Down
12 changes: 1 addition & 11 deletions packages/core/src/shared/logger/toolkitLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 } } = {}
Expand Down Expand Up @@ -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
}
Expand Down
34 changes: 14 additions & 20 deletions packages/core/src/test/shared/logger/toolkitLogger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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'

Expand Down

0 comments on commit b364573

Please sign in to comment.