From 135c3e5a73bda9c51da47eb5004b6a6d7741ed29 Mon Sep 17 00:00:00 2001 From: Nikolas Komonen <118216176+nkomonen-amazon@users.noreply.github.com> Date: Mon, 18 Nov 2024 09:52:39 -0500 Subject: [PATCH 1/7] refactor(telemetry): add functionality to @withTelemetryContext (#5816) See each commit for the specific change. But this has multiple improvements to the `@withTelemetryContext` decorator. The main change to note: - Adding the `@withTelemetryContext` decorator to a method can wrap thrown errors with context about that function. This helps us to build some sort of stack trace in the `reasonDesc` of our telemetry when errors are thrown. - [This is the documentation updated to get a better idea of what this is](https://github.com/aws/aws-toolkit-vscode/blob/b3f2ad9c65b47cd61ca72652171e932ef2480d8c/docs/telemetry.md#adding-a-stack-trace-to-your-metric) - The decorator allows for minimal diffs to the code. It is replacing [this previous code](https://github.com/aws/aws-toolkit-vscode/blob/2a72e6df814e3cbabce1cfe2476f1ca98ea8de2b/packages/core/src/shared/crashMonitoring.ts#L651) which would cause a diff in the column indentation since all the method code needed to be in a callback --- License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: nkomonen-amazon --- docs/telemetry.md | 82 +++++++------ packages/core/src/shared/telemetry/util.ts | 53 ++++++-- .../src/shared/telemetry/vscodeTelemetry.json | 4 + .../src/test/shared/telemetry/spans.test.ts | 115 ++++++++++++++++-- 4 files changed, 201 insertions(+), 53 deletions(-) diff --git a/docs/telemetry.md b/docs/telemetry.md index 0554722b6a6..7750750c967 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -142,20 +142,23 @@ Finally, if `setupStep2()` was the thing that failed we would see a metric like: ## Adding a "Stack Trace" to your metric -### Problem +When errors are thrown we do not attach the stack trace in telemetry. We only know about the error itself, but +not the path it took to get there. We sometimes need this stack trace to debug, and only have telemetry to get insight on what happened since we do not have access to logs. + +### Scenario Common example: _"I have a function, `thisFailsSometimes()` that is called in multiple places. The function sometimes fails, I know from telemetry, but I do not know if it is failing when it is a specific caller. If I knew the call stack/trace that it took to call my function that would help me debug."_ ```typescript -function outerA() { +function runsSuccessfully() { thisFailsSometimes(1) // this succeeds } -function outerB() { +function thisThrows() { thisFailsSometimes(0) // this fails } -function thisFailsSometimes(num: number) { +function failsDependingOnInput(num: number) { return telemetry.my_Metric.run(() => { if (number === 0) { throw Error('Cannot be 0') @@ -167,31 +170,61 @@ function thisFailsSometimes(num: number) { ### Solution -Add a value to `function` in the options of a `run()`. This will result in a stack of functions identifiers that were previously called -before `thisFailsSometimes()` was run. You can then retrieve the stack in the `run()` of your final metric using `getFunctionStack()`. +On class methods, use the `@withTelemetryContext()` decorator to add context to the execution. Depending on the args set, it provides features like emitting the result, or adding it's context to errors. + +> NOTE: Decorators are currently only supported for methods and not functions + +```typescript +class MyClass { + @withTelemetryContext({ name: 'runsSuccessfully', class: 'MyClass' }) + public runsSuccessfully() { + failsDependingOnInput(1) + } + + @withTelemetryContext({ name: 'thisThrows', class: 'MyClass', errorCtx: true }) + public thisThrows() { + failsDependingOnInput(0) + } + + @withTelemetryContext({ name: 'failsDependingOnInput' class: 'MyClass', emit: true, errorCtx: true}) + private failsDependingOnInput(num: number) { + if (number === 0) { + throw Error('Cannot be 0') + } + ... + } +} + +// Results in a metric: { source: 'MyClass#thisThrows,failsDependingOnInput', result: 'Failed' } +// Results in an error that has context about the methods that lead up to it. +new MyClass().thisThrows() +``` + +Separately if you must use a function, add a value to `function` in the options of a `run()`. This will result in a stack of functions identifiers that were previously called +before `failsDependingOnInput()` was run. You can then retrieve the stack in the `run()` of your final metric using `getFunctionStack()`. ```typescript -function outerA() { - telemetry.my_Metric.run(() => thisFailsSometimes(1), { functionId: { name: 'outerA' }}) +function runsSuccessfully() { + telemetry.my_Metric.run(() => failsDependingOnInput(1), { functionId: { name: 'runsSuccessfully' }}) } -function outerB() { - telemetry.my_Metric.run(() => thisFailsSometimes(0), { functionId: { source: 'outerB' }}) +function thisThrows() { + telemetry.my_Metric.run(() => failsDependingOnInput(0), { functionId: { source: 'thisThrows' }}) } -function thisFailsSometimes(num: number) { +function failsDependingOnInput(num: number) { return telemetry.my_Metric.run(() => { telemetry.record({ theCallStack: asStringifiedStack(telemetry.getFunctionStack())}) if (number === 0) { throw Error('Cannot be 0') } ... - }, { functionId: { name: 'thisFailsSometimes' }}) + }, { functionId: { name: 'failsDependingOnInput' }}) } -// Results in a metric: { theCallStack: 'outerB:thisFailsSometimes', result: 'Failed' } -// { theCallStack: 'outerB:thisFailsSometimes' } implies 'outerB' was run first, then 'thisFailsSometimes'. See docstrings for more info. -outerB() +// Results in a metric: { theCallStack: 'thisThrows:failsDependingOnInput', result: 'Failed' } +// { theCallStack: 'thisThrows:failsDependingOnInput' } implies 'thisThrows' was run first, then 'failsDependingOnInput'. See docstrings for more info. +thisThrows() ``` ### Important Notes @@ -216,25 +249,6 @@ outerB() c() // result: 'a:c', note that 'b' is not included ``` -- If you are using `run()` with a class method, you can also add the class to the entry for more context - - ```typescript - class A { - a() { - return telemetry.my_Metric.run(() => this.b(), { functionId: { name: 'a', class: 'A' } }) - } - - b() { - return telemetry.my_Metric.run(() => asStringifiedStack(telemetry.getFunctionStack()), { - functionId: { name: 'b', class: 'A' }, - }) - } - } - - const inst = new A() - inst.a() // 'A#a,b' - ``` - - If you do not want your `run()` to emit telemetry, set `emit: false` in the options ```typescript diff --git a/packages/core/src/shared/telemetry/util.ts b/packages/core/src/shared/telemetry/util.ts index 4a9a2f1ff1f..67a2d460dd8 100644 --- a/packages/core/src/shared/telemetry/util.ts +++ b/packages/core/src/shared/telemetry/util.ts @@ -26,9 +26,10 @@ import { isValidationExemptMetric } from './exemptMetrics' import { isAmazonQ, isCloud9, isSageMaker } from '../../shared/extensionUtilities' import { isUuid, randomUUID } from '../crypto' import { ClassToInterfaceType } from '../utilities/tsUtils' -import { FunctionEntry, type TelemetryTracer } from './spans' +import { asStringifiedStack, FunctionEntry } from './spans' import { telemetry } from './telemetry' import { v5 as uuidV5 } from 'uuid' +import { ToolkitError } from '../errors' const legacySettingsTelemetryValueDisable = 'Disable' const legacySettingsTelemetryValueEnable = 'Enable' @@ -365,9 +366,10 @@ export function getOperatingSystem(): OperatingSystem { } } +type TelemetryContextArgs = FunctionEntry & { emit?: boolean; errorCtx?: boolean } /** * Decorator that simply wraps the method with a non-emitting telemetry `run()`, automatically - * `record()`ing the provided function id for later use by {@link TelemetryTracer.getFunctionStack()} + * `record()`ing the provided function id for later use by TelemetryTracer.getFunctionStack() * * This saves us from needing to wrap the entire function: * @@ -393,25 +395,60 @@ export function getOperatingSystem(): OperatingSystem { * } * } * ``` + * + * @param opts.name The name of the function + * @param opts.class The class name of the function + * @param opts.emit Whether or not to emit the telemetry event (default: false) + * @param opts.errorCtx Whether or not to add the error context to the error (default: false) */ -export function withTelemetryContext(functionId: FunctionEntry) { +export function withTelemetryContext(opts: TelemetryContextArgs) { + const shouldErrorCtx = opts.errorCtx !== undefined ? opts.errorCtx : false function decorator( originalMethod: (this: This, ...args: Args) => Return, _context: ClassMethodDecoratorContext // we dont need this currently but it keeps the compiler happy ) { function decoratedMethod(this: This, ...args: Args): Return { return telemetry.function_call.run( - () => { - // DEVELOPERS: Set a breakpoint here and step in to it to debug the original function - return originalMethod.call(this, ...args) + (span) => { + try { + span.record({ + functionName: opts.name, + className: opts.class, + source: asStringifiedStack(telemetry.getFunctionStack()), + }) + + // DEVELOPERS: Set a breakpoint here and step in and debug the original function + const result = originalMethod.call(this, ...args) + + if (result instanceof Promise) { + return result.catch((e) => { + if (shouldErrorCtx) { + throw addContextToError(e, opts) + } + throw e + }) as Return + } + return result + } catch (e) { + if (shouldErrorCtx) { + throw addContextToError(e, opts) + } + throw e + } }, { - emit: false, - functionId: functionId, + emit: opts.emit !== undefined ? opts.emit : false, + functionId: { name: opts.name, class: opts.class }, } ) } return decoratedMethod } return decorator + + function addContextToError(e: unknown, functionId: FunctionEntry) { + return ToolkitError.chain(e, `ctx: ${functionId.name}`, { + code: functionId.class, + }) + } } diff --git a/packages/core/src/shared/telemetry/vscodeTelemetry.json b/packages/core/src/shared/telemetry/vscodeTelemetry.json index 5712b313eaf..bfdf4dc96c2 100644 --- a/packages/core/src/shared/telemetry/vscodeTelemetry.json +++ b/packages/core/src/shared/telemetry/vscodeTelemetry.json @@ -1137,6 +1137,10 @@ { "type": "className", "required": false + }, + { + "type": "source", + "required": false } ], "passive": true diff --git a/packages/core/src/test/shared/telemetry/spans.test.ts b/packages/core/src/test/shared/telemetry/spans.test.ts index 149fe5aa14e..8ef782943c1 100644 --- a/packages/core/src/test/shared/telemetry/spans.test.ts +++ b/packages/core/src/test/shared/telemetry/spans.test.ts @@ -4,7 +4,7 @@ */ import assert from 'assert' -import { ToolkitError } from '../../../shared/errors' +import { getErrorId, ToolkitError } from '../../../shared/errors' import { asStringifiedStack, FunctionEntry, TelemetrySpan, TelemetryTracer } from '../../../shared/telemetry/spans' import { MetricName, MetricShapes, telemetry } from '../../../shared/telemetry/telemetry' import { assertTelemetry, getMetrics, installFakeClock } from '../../testUtil' @@ -575,19 +575,112 @@ describe('TelemetryTracer', function () { 'A#a1,a2:B#b1,b2' ) }) + }) + }) - class TestEmit { - @withTelemetryContext({ name: 'doesNotEmit', class: 'TestEmit' }) - doesNotEmit() { - return - } + describe('withTelemetryContext', async function () { + class TestEmit { + @withTelemetryContext({ name: 'doesNotEmit', class: 'TestEmit' }) + doesNotEmit() { + return } - it(`withTelemetryContext does not emit an event on its own`, function () { - const inst = new TestEmit() - inst.doesNotEmit() - assertTelemetry('function_call', []) - }) + @withTelemetryContext({ name: 'doesEmit', class: 'TestEmit', emit: false }) + doesEmit() { + return this.doesEmitNested() + } + + @withTelemetryContext({ name: 'doesEmitNested', class: 'TestEmit', emit: true }) + doesEmitNested() { + return + } + } + + it(`does NOT emit an event if not explicitly set`, function () { + const inst = new TestEmit() + inst.doesNotEmit() + assertTelemetry('function_call', []) + }) + + it(`does emit an event on its own when explicitly set`, function () { + const inst = new TestEmit() + inst.doesEmit() + assertTelemetry('function_call', [ + { + functionName: 'doesEmitNested', + className: 'TestEmit', + source: 'TestEmit#doesEmit,doesEmitNested', + }, + ]) + }) + + class TestThrows { + @withTelemetryContext({ name: 'throwsError', class: 'TestThrows', errorCtx: true }) + throwsError() { + throw arbitraryError + } + + @withTelemetryContext({ name: 'throwsError', errorCtx: true }) + throwsErrorButNoClass() { + throw arbitraryError + } + + @withTelemetryContext({ name: 'throwsError', errorCtx: true }) + async throwsAsyncError() { + throw arbitraryError + } + + @withTelemetryContext({ name: 'throwsErrorButNoCtx', class: 'TestThrows' }) + throwsErrorButNoCtx() { + throw arbitraryError + } + } + const arbitraryError = new Error('arbitrary error') + + it(`wraps errors with function id context`, async function () { + const inst = new TestThrows() + assert.throws( + () => inst.throwsError(), + (e) => { + return ( + e instanceof ToolkitError && + getErrorId(e) === 'TestThrows' && + e.message === 'ctx: throwsError' && + e.cause === arbitraryError + ) + } + ) + assert.throws( + () => inst.throwsErrorButNoClass(), + (e) => { + return ( + e instanceof ToolkitError && + getErrorId(e) === 'Error' && + e.message === 'ctx: throwsError' && + e.cause === arbitraryError + ) + } + ) + await assert.rejects( + () => inst.throwsAsyncError(), + (e) => { + return ( + e instanceof ToolkitError && + getErrorId(e) === 'Error' && + e.message === 'ctx: throwsError' && + e.cause === arbitraryError + ) + } + ) + }) + + it('does not add error context by default', async function () { + const inst = new TestThrows() + + assert.throws( + () => inst.throwsErrorButNoCtx(), + (e) => e === arbitraryError + ) }) }) From 4158d67e2a9d73155d49b1068804777c29c440b4 Mon Sep 17 00:00:00 2001 From: Maxim Hayes <149123719+hayemaxi@users.noreply.github.com> Date: Mon, 18 Nov 2024 13:28:17 -0500 Subject: [PATCH 2/7] fix(notifications): update types, field names, cross-window syncing (#6040) - Make types more granular. - Align names - fix onRecieve typo, openTxt -> openTextDocument - Sync from global state on reload to detect activity in other windows. This makes it so that if users dismiss in one window, it will dismiss in all windows. It will also prevent a bug where users may get dismissed notifications again if they have multiple windows. --- License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --- packages/core/src/auth/auth.ts | 2 - packages/core/src/auth/connection.ts | 6 ++ packages/core/src/auth/sso/validation.ts | 4 +- .../util/supplementalContext/rankBm25.ts | 2 +- packages/core/src/notifications/activation.ts | 3 +- packages/core/src/notifications/controller.ts | 40 +++++++++--- packages/core/src/notifications/panelNode.ts | 9 +-- packages/core/src/notifications/rules.ts | 4 +- packages/core/src/notifications/types.ts | 62 +++++++++++++++---- .../src/test/awsService/ec2/model.test.ts | 2 +- .../src/test/notifications/controller.test.ts | 4 +- .../src/test/notifications/rendering.test.ts | 12 ++-- .../resources/emergency/1.x.json | 4 +- .../notifications/resources/startup/1.x.json | 4 +- .../core/src/test/notifications/rules.test.ts | 10 +-- 15 files changed, 119 insertions(+), 49 deletions(-) diff --git a/packages/core/src/auth/auth.ts b/packages/core/src/auth/auth.ts index 334c3841216..3335f8ab333 100644 --- a/packages/core/src/auth/auth.ts +++ b/packages/core/src/auth/auth.ts @@ -126,8 +126,6 @@ export interface ConnectionStateChangeEvent { readonly state: ProfileMetadata['connectionState'] } -export type AuthType = Auth - export type DeletedConnection = { connId: Connection['id']; storedProfile?: StoredProfile } type DeclaredConnection = Pick & { source: string } diff --git a/packages/core/src/auth/connection.ts b/packages/core/src/auth/connection.ts index 5fc505a0b5f..3e7752dd8e9 100644 --- a/packages/core/src/auth/connection.ts +++ b/packages/core/src/auth/connection.ts @@ -24,6 +24,7 @@ const warnOnce = onceChanged((s: string, url: string) => { void showMessageWithUrl(s, url, undefined, 'warn') }) +// TODO: Refactor all scopes to a central file with minimal dependencies. export const scopesCodeCatalyst = ['codecatalyst:read_write'] export const scopesSsoAccountAccess = ['sso:account:access'] /** These are the non-chat scopes for CW. */ @@ -39,6 +40,11 @@ type SsoType = | 'idc' // AWS Identity Center | 'builderId' +// TODO: This type is not centralized and there are many routines in the codebase that use some +// variation for these for validation, telemetry, UX, etc. A refactor is needed to align these +// string types. +export type AuthType = 'credentials' | 'builderId' | 'identityCenter' | 'unknown' + export const isIamConnection = (conn?: Connection): conn is IamConnection => conn?.type === 'iam' export const isSsoConnection = (conn?: Connection, type: SsoType = 'any'): conn is SsoConnection => { if (conn?.type !== 'sso') { diff --git a/packages/core/src/auth/sso/validation.ts b/packages/core/src/auth/sso/validation.ts index ecfe935a837..cad6aa7f4c3 100644 --- a/packages/core/src/auth/sso/validation.ts +++ b/packages/core/src/auth/sso/validation.ts @@ -4,7 +4,7 @@ */ import * as vscode from 'vscode' import { UnknownError } from '../../shared/errors' -import { AuthType } from '../auth' +import { Auth } from '../auth' import { SsoConnection, hasScopes, isAnySsoConnection } from '../connection' import { ssoUrlFormatMessage, ssoUrlFormatRegex } from './constants' @@ -19,7 +19,7 @@ export function validateSsoUrlFormat(url: string) { } export async function validateIsNewSsoUrlAsync( - auth: AuthType, + auth: Auth, url: string, requiredScopes?: string[] ): Promise { diff --git a/packages/core/src/codewhisperer/util/supplementalContext/rankBm25.ts b/packages/core/src/codewhisperer/util/supplementalContext/rankBm25.ts index a6fd84aafd6..3e00013c9cf 100644 --- a/packages/core/src/codewhisperer/util/supplementalContext/rankBm25.ts +++ b/packages/core/src/codewhisperer/util/supplementalContext/rankBm25.ts @@ -7,7 +7,7 @@ export interface BM25Document { content: string - /** The score that the document recieves. */ + /** The score that the document receives. */ score: number index: number diff --git a/packages/core/src/notifications/activation.ts b/packages/core/src/notifications/activation.ts index 2ac36bcd5bd..cd732909d16 100644 --- a/packages/core/src/notifications/activation.ts +++ b/packages/core/src/notifications/activation.ts @@ -11,9 +11,10 @@ import { RuleEngine, getRuleContext } from './rules' import globals from '../shared/extensionGlobals' import { AuthState } from './types' import { getLogger } from '../shared/logger/logger' +import { oneMinute } from '../shared/datetime' /** Time in MS to poll for emergency notifications */ -const emergencyPollTime = 1000 * 10 * 60 +const emergencyPollTime = oneMinute * 10 /** * Activate the in-IDE notifications module and begin receiving notifications. diff --git a/packages/core/src/notifications/controller.ts b/packages/core/src/notifications/controller.ts index a93835d89b1..30aa8ef3570 100644 --- a/packages/core/src/notifications/controller.ts +++ b/packages/core/src/notifications/controller.ts @@ -9,6 +9,7 @@ import globals from '../shared/extensionGlobals' import { globalKey } from '../shared/globalState' import { getNotificationTelemetryId, + Notifications, NotificationsState, NotificationsStateConstructor, NotificationType, @@ -56,12 +57,7 @@ export class NotificationsController { } NotificationsController.#instance = this - this.state = globals.globalState.tryGet(this.storageKey, NotificationsStateConstructor, { - startUp: {}, - emergency: {}, - dismissed: [], - newlyReceived: [], - }) + this.state = this.getDefaultState() } public pollForStartUp(ruleEngine: RuleEngine) { @@ -74,6 +70,9 @@ export class NotificationsController { private async poll(ruleEngine: RuleEngine, category: NotificationType) { try { + // Get latest state in case it was modified by other windows. + // It is a minimal read to avoid race conditions. + this.readState() await this.fetchNotifications(category) } catch (err: any) { getLogger('notifications').error(`Unable to fetch %s notifications: %s`, category, err) @@ -139,7 +138,7 @@ export class NotificationsController { return } // Parse the notifications - const newPayload = JSON.parse(response.content) + const newPayload: Notifications = JSON.parse(response.content) const newNotifications = newPayload.notifications ?? [] // Get the current notifications @@ -188,6 +187,33 @@ export class NotificationsController { await globals.globalState.update(this.storageKey, this.state) } + /** + * Read relevant values from the latest global state to memory. Useful to bring changes from other windows. + * + * Currently only brings dismissed, so users with multiple vscode instances open do not have issues with + * dismissing notifications multiple times. Otherwise, each instance has an independent session for + * displaying the notifications (e.g. multiple windows can be blocked in critical emergencies). + * + * Note: This sort of pattern (reading back and forth from global state in async functions) is prone to + * race conditions, which is why we limit the read to the fairly inconsequential `dismissed` property. + */ + private readState() { + const state = this.getDefaultState() + this.state.dismissed = state.dismissed + } + + /** + * Returns stored notification state, or a default state object if it is invalid or undefined. + */ + private getDefaultState() { + return globals.globalState.tryGet(this.storageKey, NotificationsStateConstructor, { + startUp: {}, + emergency: {}, + dismissed: [], + newlyReceived: [], + }) + } + static get instance() { if (this.#instance === undefined) { throw new ToolkitError('NotificationsController was accessed before it has been initialized.') diff --git a/packages/core/src/notifications/panelNode.ts b/packages/core/src/notifications/panelNode.ts index 94e4f2031f1..fe2512be47b 100644 --- a/packages/core/src/notifications/panelNode.ts +++ b/packages/core/src/notifications/panelNode.ts @@ -159,7 +159,8 @@ export class NotificationsNode implements TreeNode { * instructions included in the notification. See {@link ToolkitNotification.uiRenderInstructions}. */ public async openNotification(notification: ToolkitNotification) { - switch (notification.uiRenderInstructions.onClick.type) { + const onClickType = notification.uiRenderInstructions.onClick.type + switch (onClickType) { case 'modal': // Render blocking modal logger.verbose(`rendering modal for notificaiton: ${notification.id} ...`) @@ -180,7 +181,7 @@ export class NotificationsNode implements TreeNode { // Display read-only txt document logger.verbose(`showing txt document for notification: ${notification.id} ...`) await telemetry.toolkit_invokeAction.run(async () => { - telemetry.record({ source: getNotificationTelemetryId(notification), action: 'openTxt' }) + telemetry.record({ source: getNotificationTelemetryId(notification), action: onClickType }) await readonlyDocument.show( notification.uiRenderInstructions.content['en-US'].description, `Notification: ${notification.id}` @@ -230,7 +231,7 @@ export class NotificationsNode implements TreeNode { // Different button options if (selectedButton) { switch (selectedButton.type) { - case 'openTxt': + case 'openTextDocument': await readonlyDocument.show( notification.uiRenderInstructions.content['en-US'].description, `Notification: ${notification.id}` @@ -260,7 +261,7 @@ export class NotificationsNode implements TreeNode { public async onReceiveNotifications(notifications: ToolkitNotification[]) { for (const notification of notifications) { - void this.showInformationWindow(notification, notification.uiRenderInstructions.onRecieve, true) + void this.showInformationWindow(notification, notification.uiRenderInstructions.onReceive, true) } } diff --git a/packages/core/src/notifications/rules.ts b/packages/core/src/notifications/rules.ts index 9e62adb7cc1..34f5660a50e 100644 --- a/packages/core/src/notifications/rules.ts +++ b/packages/core/src/notifications/rules.ts @@ -10,6 +10,7 @@ import { ConditionalClause, RuleContext, DisplayIf, CriteriaCondition, ToolkitNo import { getComputeEnvType, getOperatingSystem } from '../shared/telemetry/util' import { AuthFormId } from '../login/webview/vue/types' import { getLogger } from '../shared/logger/logger' +import { ToolkitError } from '../shared/errors' const logger = getLogger('notifications') /** @@ -158,7 +159,8 @@ export class RuleEngine { case 'ActiveExtensions': return isSuperSetOfExpected(this.context.activeExtensions) default: - throw new Error(`Unknown criteria type: ${criteria.type}`) + logger.error('Unknown criteria passed to RuleEngine: %O', criteria) + throw new ToolkitError(`Unknown criteria type: ${(criteria as any).type}`) } } } diff --git a/packages/core/src/notifications/types.ts b/packages/core/src/notifications/types.ts index 59717cf9079..5ee06df5cd9 100644 --- a/packages/core/src/notifications/types.ts +++ b/packages/core/src/notifications/types.ts @@ -7,16 +7,55 @@ import * as vscode from 'vscode' import { EnvType, OperatingSystem } from '../shared/telemetry/util' import { TypeConstructor } from '../shared/utilities/typeConstructors' import { AuthUserState, AuthStatus } from '../shared/telemetry/telemetry.gen' +import { AuthType } from '../auth/connection' /** Types of information that we can use to determine whether to show a notification or not. */ -export type Criteria = 'OS' | 'ComputeEnv' | 'AuthType' | 'AuthRegion' | 'AuthState' | 'AuthScopes' | 'ActiveExtensions' -/** Generic condition where the type determines how the values are evaluated. */ -export interface CriteriaCondition { - readonly type: Criteria - readonly values: string[] +type OsCriteria = { + type: 'OS' + values: OperatingSystem[] +} + +type ComputeEnvCriteria = { + type: 'ComputeEnv' + values: EnvType[] +} + +type AuthTypeCriteria = { + type: 'AuthType' + values: AuthType[] +} + +type AuthRegionCriteria = { + type: 'AuthRegion' + values: string[] +} + +type AuthStateCriteria = { + type: 'AuthState' + values: AuthStatus[] +} + +type AuthScopesCriteria = { + type: 'AuthScopes' + values: string[] // TODO: Scopes should be typed. Could import here, but don't want to import too much. +} + +type ActiveExtensionsCriteria = { + type: 'ActiveExtensions' + values: string[] } +/** Generic condition where the type determines how the values are evaluated. */ +export type CriteriaCondition = + | OsCriteria + | ComputeEnvCriteria + | AuthTypeCriteria + | AuthRegionCriteria + | AuthStateCriteria + | AuthScopesCriteria + | ActiveExtensionsCriteria + /** One of the subconditions (clauses) must match to be valid. */ export interface OR { readonly type: 'or' @@ -39,8 +78,8 @@ export interface ExactMatch { export type ConditionalClause = Range | ExactMatch | OR export type OnReceiveType = 'toast' | 'modal' -export type OnClickType = 'modal' | 'openTextDocument' | 'openUrl' -export type ActionType = 'openUrl' | 'updateAndReload' | 'openTxt' +export type OnClickType = { type: 'modal' } | { type: 'openTextDocument' } | { type: 'openUrl'; url: string } +export type ActionType = 'openUrl' | 'updateAndReload' | 'openTextDocument' /** How to display the notification. */ export interface UIRenderInstructions { @@ -51,11 +90,8 @@ export interface UIRenderInstructions { toastPreview?: string // optional property for toast } } - onRecieve: OnReceiveType // TODO: typo - onClick: { - type: OnClickType - url?: string // optional property for 'openUrl' - } + onReceive: OnReceiveType + onClick: OnClickType actions?: Array<{ type: ActionType displayText: { @@ -124,7 +160,7 @@ export interface RuleContext { readonly extensionVersion: string readonly os: OperatingSystem readonly computeEnv: EnvType - readonly authTypes: ('credentials' | 'builderId' | 'identityCenter' | 'unknown')[] + readonly authTypes: AuthType[] readonly authRegions: string[] readonly authStates: AuthStatus[] readonly authScopes: string[] diff --git a/packages/core/src/test/awsService/ec2/model.test.ts b/packages/core/src/test/awsService/ec2/model.test.ts index 6c7f6f1fb4a..d39de4ba1da 100644 --- a/packages/core/src/test/awsService/ec2/model.test.ts +++ b/packages/core/src/test/awsService/ec2/model.test.ts @@ -24,7 +24,7 @@ describe('Ec2ConnectClient', function () { }) describe('getAttachedIamRole', async function () { - it('only returns role if recieves ARN from instance profile', async function () { + it('only returns role if receives ARN from instance profile', async function () { let role: IAM.Role | undefined const getInstanceProfileStub = sinon.stub(Ec2Client.prototype, 'getAttachedIamInstanceProfile') diff --git a/packages/core/src/test/notifications/controller.test.ts b/packages/core/src/test/notifications/controller.test.ts index c1e2d299463..25b013db71b 100644 --- a/packages/core/src/test/notifications/controller.test.ts +++ b/packages/core/src/test/notifications/controller.test.ts @@ -557,7 +557,7 @@ function getValidTestNotification(id: string): ToolkitNotification { description: 'test', }, }, - onRecieve: 'toast', + onReceive: 'toast', onClick: { type: 'openUrl', url: 'https://aws.amazon.com/visualstudiocode/', @@ -580,7 +580,7 @@ function getInvalidTestNotification(id: string): ToolkitNotification { description: 'test', }, }, - onRecieve: 'toast', + onReceive: 'toast', onClick: { type: 'modal' }, }, } diff --git a/packages/core/src/test/notifications/rendering.test.ts b/packages/core/src/test/notifications/rendering.test.ts index e446e53fdba..ccda76195a2 100644 --- a/packages/core/src/test/notifications/rendering.test.ts +++ b/packages/core/src/test/notifications/rendering.test.ts @@ -112,7 +112,7 @@ describe('Notifications Rendering', function () { assert.ok(telemetrySpy.calledOnce) }) - it('executes openURL type button', async function () { + it('executes openUrl type button', async function () { const testWindow = getTestWindow() testWindow.onDidShowMessage((message) => { // Simulate user clicking open URL type @@ -122,7 +122,7 @@ describe('Notifications Rendering', function () { await verifyOpenExternalUrl(notification) }) - it('executes openTxt type button', async function () { + it('executes openTextDocument type button', async function () { const testWindow = getTestWindow() testWindow.onDidShowMessage((message) => { // Simulate user clicking open txt type @@ -148,7 +148,7 @@ function getToastURLTestNotification(): ToolkitNotification { toastPreview: 'test toast preview', }, }, - onRecieve: 'toast', + onReceive: 'toast', onClick: { type: 'openUrl', url: 'https://aws.amazon.com/visualstudiocode/', @@ -170,7 +170,7 @@ function getTxtNotification(): ToolkitNotification { description: 'This is a text document notification.', }, }, - onRecieve: 'toast', + onReceive: 'toast', onClick: { type: 'openTextDocument', }, @@ -191,7 +191,7 @@ function getModalNotification(): ToolkitNotification { description: 'This is a modal notification.', }, }, - onRecieve: 'modal', + onReceive: 'modal', onClick: { type: 'modal', }, @@ -210,7 +210,7 @@ function getModalNotification(): ToolkitNotification { }, }, { - type: 'openTxt', + type: 'openTextDocument', displayText: { 'en-US': 'Read More', }, diff --git a/packages/core/src/test/notifications/resources/emergency/1.x.json b/packages/core/src/test/notifications/resources/emergency/1.x.json index ced10965ab5..b0a83f40d3b 100644 --- a/packages/core/src/test/notifications/resources/emergency/1.x.json +++ b/packages/core/src/test/notifications/resources/emergency/1.x.json @@ -14,7 +14,7 @@ "toastPreview": "Signing into Amazon Q is broken, please try this workaround while we work on releasing a fix." } }, - "onRecieve": "toast", + "onReceive": "toast", "onClick": { "type": "openTextDocument" }, @@ -34,7 +34,7 @@ "toastPreview": "This version of Amazon Q is currently broken, please update to avoid issues." } }, - "onRecieve": "toast", + "onReceive": "toast", "onClick": { "type": "modal" }, diff --git a/packages/core/src/test/notifications/resources/startup/1.x.json b/packages/core/src/test/notifications/resources/startup/1.x.json index 29a7e7e89cd..aae5b1d70f2 100644 --- a/packages/core/src/test/notifications/resources/startup/1.x.json +++ b/packages/core/src/test/notifications/resources/startup/1.x.json @@ -14,7 +14,7 @@ "toastPreview": "New Amazon Q features available: inline chat" } }, - "onRecieve": "toast", + "onReceive": "toast", "onClick": { "type": "openTextDocument" }, @@ -41,7 +41,7 @@ "toastPreview": "New Amazon Q features are available!" } }, - "onRecieve": "toast", + "onReceive": "toast", "onClick": { "type": "openUrl", "url": "https://aws.amazon.com/developer/generative-ai/amazon-q/change-log/" diff --git a/packages/core/src/test/notifications/rules.test.ts b/packages/core/src/test/notifications/rules.test.ts index 867b2119546..0df695c1be6 100644 --- a/packages/core/src/test/notifications/rules.test.ts +++ b/packages/core/src/test/notifications/rules.test.ts @@ -40,7 +40,7 @@ describe('Notifications Rule Engine', function () { description: 'Something crazy is happening! Please update your extension.', }, }, - onRecieve: 'toast', + onReceive: 'toast', onClick: { type: 'openUrl', url: 'https://aws.amazon.com/visualstudiocode/', @@ -305,7 +305,7 @@ describe('Notifications Rule Engine', function () { assert.equal( ruleEngine.shouldDisplayNotification( buildNotification({ - additionalCriteria: [{ type: 'AuthType', values: ['builderId', 'iamIdentityCenter'] }], + additionalCriteria: [{ type: 'AuthType', values: ['builderId', 'identityCenter'] }], }) ), true @@ -316,7 +316,7 @@ describe('Notifications Rule Engine', function () { assert.equal( ruleEngine.shouldDisplayNotification( buildNotification({ - additionalCriteria: [{ type: 'AuthType', values: ['iamIdentityCenter'] }], + additionalCriteria: [{ type: 'AuthType', values: ['identityCenter'] }], }) ), false @@ -452,7 +452,7 @@ describe('Notifications Rule Engine', function () { additionalCriteria: [ { type: 'OS', values: ['LINUX', 'MAC'] }, { type: 'ComputeEnv', values: ['local', 'ec2'] }, - { type: 'AuthType', values: ['builderId', 'iamIdentityCenter'] }, + { type: 'AuthType', values: ['builderId', 'identityCenter'] }, { type: 'AuthRegion', values: ['us-east-1', 'us-west-2'] }, { type: 'AuthState', values: ['connected'] }, { type: 'AuthScopes', values: ['codewhisperer:completions', 'codewhisperer:analysis'] }, @@ -489,7 +489,7 @@ describe('Notifications Rule Engine', function () { }, additionalCriteria: [ { type: 'OS', values: ['LINUX', 'MAC'] }, - { type: 'AuthType', values: ['builderId', 'iamIdentityCenter'] }, + { type: 'AuthType', values: ['builderId', 'identityCenter'] }, { type: 'AuthRegion', values: ['us-east-1', 'us-west-2'] }, { type: 'AuthState', values: ['connected'] }, { type: 'AuthScopes', values: ['codewhisperer:completions', 'codewhisperer:analysis'] }, From ccba8192d4a097bef486d441ae38516db7c42b67 Mon Sep 17 00:00:00 2001 From: Hweinstock <42325418+Hweinstock@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:12:01 -0500 Subject: [PATCH 3/7] build(test): add automatic retry to all mocha tests. (#6005) ## Problem There are many unreliable tests that seem to fail very rarely or even only once. If they are truly failing so rarely, retrying them should keep CI green. This also allows us to avoid investigating flaky tests that aren't having a meaningful negative impact on the dev experience. ## Solution - retry failed tests up to 3 times. --- License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --- packages/core/src/test/testRunner.ts | 9 +++++---- packages/toolkit/test/unit/index.ts | 9 ++++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/core/src/test/testRunner.ts b/packages/core/src/test/testRunner.ts index ecac03475b7..9ba9c7cd01e 100644 --- a/packages/core/src/test/testRunner.ts +++ b/packages/core/src/test/testRunner.ts @@ -22,7 +22,7 @@ export async function runTests( testFolder: string | string[], extensionId: string, initTests: string[] = [], - testFiles?: string[] + options?: { retries?: number; testFiles?: string[] } ): Promise { if (!process.env['AWS_TOOLKIT_AUTOMATION']) { throw new Error('Expected the "AWS_TOOLKIT_AUTOMATION" environment variable to be set for tests.') @@ -79,6 +79,7 @@ export async function runTests( mochaFile: outputFile, }, }, + retries: options?.retries ?? 0, timeout: 0, }) @@ -91,7 +92,7 @@ export async function runTests( testFilePath = testFile ? path.resolve(dist, testFile) : undefined } - if (testFile && testFiles) { + if (testFile && options?.testFiles) { throw new Error('Individual file and list of files given to run tests on. One must be chosen.') } @@ -143,8 +144,8 @@ export async function runTests( } let files: string[] = [] - if (testFiles) { - files = testFiles + if (options?.testFiles) { + files = options.testFiles } else { for (const f of Array.isArray(testFolder) ? testFolder : [testFolder]) { files = [...files, ...(await glob(testFilePath ?? `**/${f}/**/**.test.js`, { cwd: dist }))] diff --git a/packages/toolkit/test/unit/index.ts b/packages/toolkit/test/unit/index.ts index 998953c7fd4..6f2052d5647 100644 --- a/packages/toolkit/test/unit/index.ts +++ b/packages/toolkit/test/unit/index.ts @@ -7,7 +7,10 @@ import { runTests } from 'aws-core-vscode/test' import { VSCODE_EXTENSION_ID } from 'aws-core-vscode/utils' export function run(): Promise { - return runTests(process.env.TEST_DIR ?? ['test/unit', '../../core/dist/src/test'], VSCODE_EXTENSION_ID.awstoolkit, [ - '../../core/dist/src/test/globalSetup.test.ts', - ]) + return runTests( + process.env.TEST_DIR ?? ['test/unit', '../../core/dist/src/test'], + VSCODE_EXTENSION_ID.awstoolkit, + ['../../core/dist/src/test/globalSetup.test.ts'], + { retries: 3 } + ) } From 988dea0829437444eadeba453c670b81eb575423 Mon Sep 17 00:00:00 2001 From: Tom Zu <138054255+tomcat323@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:17:48 -0500 Subject: [PATCH 4/7] test(notifications): integ test from hosted endpoints (#6006) integ test for notifications pulling from [hosted file endpoints ](https://code.amazon.com/packages/AwsIdesAndToolsHostedFiles/trees/mainline/--/configuration/hostedFiles/Notifications/integ/VSCode) Test covers: 1. receiving JSON from endpoints 2. process notifications criterias 3. trigger onReceive for newly added notifications Also updated ruleEngine evaluation logic to skip extension id check if in test enviorments. Since we use a fake extension id for testing. --------- Co-authored-by: Maxim Hayes <149123719+hayemaxi@users.noreply.github.com> --- packages/core/src/notifications/rules.ts | 4 +- .../notifications/notifications.test.ts | 114 ++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/testInteg/notifications/notifications.test.ts diff --git a/packages/core/src/notifications/rules.ts b/packages/core/src/notifications/rules.ts index 34f5660a50e..f2d41d7bcbc 100644 --- a/packages/core/src/notifications/rules.ts +++ b/packages/core/src/notifications/rules.ts @@ -8,6 +8,7 @@ import * as semver from 'semver' import globals from '../shared/extensionGlobals' import { ConditionalClause, RuleContext, DisplayIf, CriteriaCondition, ToolkitNotification, AuthState } from './types' import { getComputeEnvType, getOperatingSystem } from '../shared/telemetry/util' +import { isAutomation } from '../shared/vscode/env' import { AuthFormId } from '../login/webview/vue/types' import { getLogger } from '../shared/logger/logger' import { ToolkitError } from '../shared/errors' @@ -79,7 +80,8 @@ export class RuleEngine { private evaluate(id: string, condition: DisplayIf): boolean { const currentExt = globals.context.extension.id - if (condition.extensionId !== currentExt) { + // if in test, skip the extension id check since its fake + if (condition.extensionId !== currentExt && !isAutomation()) { logger.verbose( 'notification id: (%s) did NOT pass extension id check, actual ext id: (%s), expected ext id: (%s)', id, diff --git a/packages/core/src/testInteg/notifications/notifications.test.ts b/packages/core/src/testInteg/notifications/notifications.test.ts new file mode 100644 index 00000000000..5b753f783ff --- /dev/null +++ b/packages/core/src/testInteg/notifications/notifications.test.ts @@ -0,0 +1,114 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 fort + */ + +import { RemoteFetcher } from '../../notifications/controller' +import { NotificationsController } from '../../notifications/controller' +import { NotificationsNode } from '../../notifications/panelNode' +import { RuleEngine } from '../../notifications/rules' +import { NotificationsState, RuleContext } from '../../notifications/types' +import globals from '../../shared/extensionGlobals' +import assert from 'assert' +import { VSCODE_EXTENSION_ID } from '../../shared/extensions' +import sinon from 'sinon' + +describe('Notifications Integration Test', function () { + let fetcher: RemoteFetcher + let panelNode: NotificationsNode + let controller: NotificationsController + + function getEngine(activeExtensions: string[], connected: boolean): RuleEngine { + const authState = connected ? 'connected' : 'notConnected' + + const ruleContext: RuleContext = { + ideVersion: '1.83.0', + extensionVersion: '1.20.0', + os: 'LINUX', + computeEnv: 'local', + authTypes: ['builderId'], + authRegions: ['us-east-2'], + authStates: [authState], + authScopes: ['codewhisperer:completions', 'codewhisperer:analysis'], + activeExtensions: activeExtensions, + } + + return new RuleEngine(ruleContext) + } + + beforeEach(async function () { + panelNode = NotificationsNode.instance + // fetch from test host file folder + fetcher = new RemoteFetcher( + 'https://idetoolkits-hostedfiles.amazonaws.com/Notifications/integ/VSCode/startup/1.x.json', + 'https://idetoolkits-hostedfiles.amazonaws.com/Notifications/integ/VSCode/emergency/1.x.json' + ) + controller = new NotificationsController(panelNode, fetcher) + }) + + // Clear all global states after each test + afterEach(async function () { + await globals.globalState.update(controller.storageKey, undefined) + }) + + it('Receive notifications polling from endpoint', async function () { + const engine = getEngine([VSCODE_EXTENSION_ID.amazonq], true) + + await controller.pollForStartUp(engine) + await controller.pollForEmergencies(engine) + + // Verify that notifications are stored in the state + const state = globals.globalState.get(controller.storageKey) + assert.ok(state, 'State should be defined') + assert.ok(state.startUp.payload?.notifications, 'StartUp received') + assert.strictEqual( + state?.startUp.payload?.notifications.length, + 2, + 'There should be 2 startup notifications stored' + ) + assert.ok(state?.emergency.payload?.notifications, 'Emergency received') + assert.strictEqual( + state?.emergency.payload?.notifications.length, + 2, + 'There should be 2 emergency notifications stored' + ) + }) + + it('Display notification according to criterias', async function () { + const engine = getEngine([VSCODE_EXTENSION_ID.amazonq], false) + await controller.pollForEmergencies(engine) + + // Verify that only the not authed notification is displayed + const displayedNotifications = panelNode.emergencyNotifications + assert.strictEqual(displayedNotifications.length, 1, 'Only one notification displayed"') + assert.strictEqual( + displayedNotifications[0].id, + 'emergency1', + 'The displayed notification have the ID "emergency1"' + ) + }) + + it('Should trigger onReceive only for newly received notifications', async function () { + const engine = getEngine([VSCODE_EXTENSION_ID.amazonq], true) + const onReceiveSpy = sinon.spy(panelNode, 'onReceiveNotifications') + + // Simulate alreadying receving a notification + const existingNotificationID = 'startup1' + const state = globals.globalState.get(controller.storageKey) + if (state) { + state.newlyReceived.push(existingNotificationID) + await globals.globalState.update(controller.storageKey, state) + } + + // Poll for new notification + await controller.pollForStartUp(engine) + + /** + * Since we simulated startup1 is already received + * onReceived should be called exactly once for startup2 + */ + assert.ok(onReceiveSpy.calledOnce, 'only one new notification received') + + onReceiveSpy.restore() + }) +}) From 87ea4d998b16fdc0a6dca82216621cf6d8171464 Mon Sep 17 00:00:00 2001 From: David <60020664+dhasani23@users.noreply.github.com> Date: Mon, 18 Nov 2024 11:26:47 -0800 Subject: [PATCH 5/7] feat(amazonq): enable SQL conversions feature (#5925) ## Problem Enable the SQL conversion feature for `/transform`. ## Solution Enable feature, intended for 11/21 release. --- License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: David Hasani --- ...-e3c11418-cbdc-4281-ab70-b1a7c574aaa1.json | 4 ++ .../src/amazonq/webview/ui/tabs/constants.ts | 11 +---- .../chat/controller/controller.ts | 47 ++++++++++++++----- .../chat/controller/messenger/messenger.ts | 27 +++-------- .../core/src/amazonqGumby/models/constants.ts | 2 +- .../commands/startTransformByQ.ts | 43 +++++++++++++++-- .../src/codewhisperer/models/constants.ts | 29 +++++++----- .../core/src/codewhisperer/models/model.ts | 2 +- .../transformByQ/transformApiHandler.ts | 39 +++++++-------- .../transformationResultsViewProvider.ts | 6 +++ packages/core/src/dev/config.ts | 3 -- .../src/shared/utilities/workspaceUtils.ts | 23 +++++++++ .../commands/transformByQ.test.ts | 14 ++++++ .../shared/utilities/workspaceUtils.test.ts | 12 +++++ 14 files changed, 177 insertions(+), 85 deletions(-) create mode 100644 packages/amazonq/.changes/next-release/Feature-e3c11418-cbdc-4281-ab70-b1a7c574aaa1.json diff --git a/packages/amazonq/.changes/next-release/Feature-e3c11418-cbdc-4281-ab70-b1a7c574aaa1.json b/packages/amazonq/.changes/next-release/Feature-e3c11418-cbdc-4281-ab70-b1a7c574aaa1.json new file mode 100644 index 00000000000..c0efd32b02c --- /dev/null +++ b/packages/amazonq/.changes/next-release/Feature-e3c11418-cbdc-4281-ab70-b1a7c574aaa1.json @@ -0,0 +1,4 @@ +{ + "type": "Feature", + "description": "Feature(Amazon Q Code Transformation): support conversions of embedded SQL from Oracle to PostgreSQL" +} diff --git a/packages/core/src/amazonq/webview/ui/tabs/constants.ts b/packages/core/src/amazonq/webview/ui/tabs/constants.ts index 2feb206f5f4..131c9b48be1 100644 --- a/packages/core/src/amazonq/webview/ui/tabs/constants.ts +++ b/packages/core/src/amazonq/webview/ui/tabs/constants.ts @@ -2,7 +2,6 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0 */ -import { isSQLTransformReady } from '../../../../dev/config' import { TabType } from '../storages/tabsStorage' import { QuickActionCommandGroup } from '@aws/mynah-ui' @@ -47,14 +46,6 @@ What would you like to work on?`, gumby: { title: 'Q - Code Transformation', placeholder: 'Open a new tab to chat with Q', - welcome: isSQLTransformReady - ? `Welcome to code transformation! - -I can help you with the following tasks: -- Upgrade your Java 8 and Java 11 codebases to Java 17 -- Convert embedded SQL from Oracle databases to PostgreSQL - -What would you like to do? You can enter 'language upgrade' or 'SQL conversion'.` - : `Welcome to code transformation!`, + welcome: 'Welcome to Code Transformation!', }, } diff --git a/packages/core/src/amazonqGumby/chat/controller/controller.ts b/packages/core/src/amazonqGumby/chat/controller/controller.ts index 1a9cd74bb73..37d139f5f1d 100644 --- a/packages/core/src/amazonqGumby/chat/controller/controller.ts +++ b/packages/core/src/amazonqGumby/chat/controller/controller.ts @@ -33,7 +33,7 @@ import { getValidSQLConversionCandidateProjects, validateSQLMetadataFile, } from '../../../codewhisperer/commands/startTransformByQ' -import { JDKVersion, transformByQState } from '../../../codewhisperer/models/model' +import { JDKVersion, TransformationCandidateProject, transformByQState } from '../../../codewhisperer/models/model' import { AbsolutePathDetectedError, AlternateDependencyVersionsNotFoundError, @@ -62,7 +62,6 @@ import { getStringHash } from '../../../shared/utilities/textUtilities' import { getVersionData } from '../../../codewhisperer/service/transformByQ/transformMavenHandler' import AdmZip from 'adm-zip' import { AuthError } from '../../../auth/sso/server' -import { isSQLTransformReady } from '../../../dev/config' // These events can be interactions within the chat, // or elsewhere in the IDE @@ -190,12 +189,31 @@ export class GumbyController { } private async transformInitiated(message: any) { - // feature flag for SQL transformations - if (!isSQLTransformReady) { + // silently check for projects eligible for SQL conversion + let embeddedSQLProjects: TransformationCandidateProject[] = [] + try { + embeddedSQLProjects = await getValidSQLConversionCandidateProjects() + } catch (err) { + getLogger().error(`CodeTransformation: error validating SQL conversion projects: ${err}`) + } + + if (embeddedSQLProjects.length === 0) { await this.handleLanguageUpgrade(message) return } + let javaUpgradeProjects: TransformationCandidateProject[] = [] + try { + javaUpgradeProjects = await getValidLanguageUpgradeCandidateProjects() + } catch (err) { + getLogger().error(`CodeTransformation: error validating Java upgrade projects: ${err}`) + } + + if (javaUpgradeProjects.length === 0) { + await this.handleSQLConversion(message) + return + } + // if previous transformation was already running, show correct message to user switch (this.sessionStorage.getSession().conversationState) { case ConversationState.JOB_SUBMITTED: @@ -224,7 +242,10 @@ export class GumbyController { this.sessionStorage.getSession().conversationState = ConversationState.WAITING_FOR_TRANSFORMATION_OBJECTIVE this.messenger.sendStaticTextResponse('choose-transformation-objective', message.tabID) this.messenger.sendChatInputEnabled(message.tabID, true) - this.messenger.sendUpdatePlaceholder(message.tabID, "Enter 'language upgrade' or 'SQL conversion'") + this.messenger.sendUpdatePlaceholder( + message.tabID, + CodeWhispererConstants.chooseTransformationObjectivePlaceholder + ) } private async beginTransformation(message: any) { @@ -310,13 +331,7 @@ export class GumbyController { private async validateSQLConversionProjects(message: any) { try { - const validProjects = await telemetry.codeTransform_validateProject.run(async () => { - telemetry.record({ - codeTransformSessionId: CodeTransformTelemetryState.instance.getSessionId(), - }) - const validProjects = await getValidSQLConversionCandidateProjects() - return validProjects - }) + const validProjects = await getValidSQLConversionCandidateProjects() return validProjects } catch (e: any) { if (e instanceof NoJavaProjectsFoundError) { @@ -624,6 +639,14 @@ export class GumbyController { case ConversationState.WAITING_FOR_TRANSFORMATION_OBJECTIVE: { const objective = data.message.trim().toLowerCase() + // since we're prompting the user, their project(s) must be eligible for both types of transformations, so track how often this happens here + if (objective === 'language upgrade' || objective === 'sql conversion') { + telemetry.codeTransform_submitSelection.emit({ + codeTransformSessionId: CodeTransformTelemetryState.instance.getSessionId(), + userChoice: objective, + result: 'Succeeded', + }) + } if (objective === 'language upgrade') { await this.handleLanguageUpgrade(data) } else if (objective === 'sql conversion') { diff --git a/packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts b/packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts index bacbbfd5bf8..0acee2a3260 100644 --- a/packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts +++ b/packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts @@ -256,7 +256,7 @@ export class Messenger { formItems.push({ id: 'GumbyTransformSQLConversionProjectForm', type: 'select', - title: 'Choose a project to transform', + title: CodeWhispererConstants.chooseProjectFormTitle, mandatory: true, options: projectFormOptions, }) @@ -264,7 +264,7 @@ export class Messenger { formItems.push({ id: 'GumbyTransformSQLSchemaForm', type: 'select', - title: 'Choose the schema of the database', + title: CodeWhispererConstants.chooseSchemaFormTitle, mandatory: true, options: Array.from(transformByQState.getSchemaOptions()).map((schema) => ({ value: schema, @@ -275,7 +275,7 @@ export class Messenger { this.dispatcher.sendAsyncEventProgress( new AsyncEventProgressMessage(tabID, { inProgress: true, - message: 'I can convert your embedded SQL, but I need some more info from you first.', + message: CodeWhispererConstants.chooseProjectSchemaFormMessage, }) ) @@ -394,7 +394,7 @@ export class Messenger { message = 'I will continue transforming your code without upgrading this dependency.' break case 'choose-transformation-objective': - message = 'Choose your transformation objective.' + message = CodeWhispererConstants.chooseTransformationObjective break } @@ -426,6 +426,7 @@ export class Messenger { message = CodeWhispererConstants.noJavaProjectsFoundChatMessage break case 'no-maven-java-project-found': + // shown when user has no pom.xml, but at this point also means they have no eligible SQL conversion projects message = CodeWhispererConstants.noPomXmlFoundChatMessage break case 'could-not-compile-project': @@ -451,23 +452,7 @@ export class Messenger { break } - const buttons: ChatItemButton[] = [] - buttons.push({ - keepCardAfterClick: false, - text: CodeWhispererConstants.startTransformationButtonText, - id: ButtonActions.CONFIRM_START_TRANSFORMATION_FLOW, - }) - - this.dispatcher.sendChatMessage( - new ChatMessage( - { - message, - messageType: 'ai-prompt', - buttons, - }, - tabID - ) - ) + this.sendJobFinishedMessage(tabID, message) } /** diff --git a/packages/core/src/amazonqGumby/models/constants.ts b/packages/core/src/amazonqGumby/models/constants.ts index 1478745497a..87f9d456690 100644 --- a/packages/core/src/amazonqGumby/models/constants.ts +++ b/packages/core/src/amazonqGumby/models/constants.ts @@ -7,6 +7,6 @@ export const gumbyChat = 'gumbyChat' // This sets the tab name -export const featureName = 'Q - Code Transform' +export const featureName = 'Q - Code Transformation' export const dependencyNoAvailableVersions = 'no available versions' diff --git a/packages/core/src/codewhisperer/commands/startTransformByQ.ts b/packages/core/src/codewhisperer/commands/startTransformByQ.ts index abcae3a1dd4..089eadd5ba1 100644 --- a/packages/core/src/codewhisperer/commands/startTransformByQ.ts +++ b/packages/core/src/codewhisperer/commands/startTransformByQ.ts @@ -20,6 +20,7 @@ import { TransformByQStatus, DB, TransformationType, + TransformationCandidateProject, } from '../models/model' import { createZipManifest, @@ -80,6 +81,8 @@ import { setContext } from '../../shared/vscode/setContext' import { makeTemporaryToolkitFolder } from '../../shared' import globals from '../../shared/extensionGlobals' import { convertDateToTimestamp } from '../../shared/datetime' +import { isWin } from '../../shared/vscode/env' +import { findStringInDirectory } from '../../shared/utilities/workspaceUtils' function getFeedbackCommentData() { const jobId = transformByQState.getJobId() @@ -150,7 +153,7 @@ export async function validateSQLMetadataFile(fileContents: string, message: any } export async function setMaven() { - let mavenWrapperExecutableName = os.platform() === 'win32' ? 'mvnw.cmd' : 'mvnw' + let mavenWrapperExecutableName = isWin() ? 'mvnw.cmd' : 'mvnw' const mavenWrapperExecutablePath = path.join(transformByQState.getProjectPath(), mavenWrapperExecutableName) if (fs.existsSync(mavenWrapperExecutablePath)) { if (mavenWrapperExecutableName === 'mvnw') { @@ -730,13 +733,45 @@ export async function finalizeTransformationJob(status: string) { export async function getValidLanguageUpgradeCandidateProjects() { const openProjects = await getOpenProjects() const javaMavenProjects = await validateOpenProjects(openProjects) + getLogger().info(`CodeTransformation: found ${javaMavenProjects.length} projects eligible for language upgrade`) return javaMavenProjects } export async function getValidSQLConversionCandidateProjects() { - const openProjects = await getOpenProjects() - const javaProjects = await getJavaProjects(openProjects) - return javaProjects + const embeddedSQLProjects: TransformationCandidateProject[] = [] + await telemetry.codeTransform_validateProject.run(async () => { + telemetry.record({ + codeTransformSessionId: CodeTransformTelemetryState.instance.getSessionId(), + }) + const openProjects = await getOpenProjects() + const javaProjects = await getJavaProjects(openProjects) + let resultLog = '' + for (const project of javaProjects) { + // as long as at least one of these strings is found, project contains embedded SQL statements + const searchStrings = ['oracle.jdbc.OracleDriver', 'jdbc:oracle:thin:@', 'jdbc:oracle:oci:@', 'jdbc:odbc:'] + for (const str of searchStrings) { + const spawnResult = await findStringInDirectory(str, project.path) + // just for telemetry purposes + if (spawnResult.error || spawnResult.stderr) { + resultLog += `search failed: ${JSON.stringify(spawnResult)}` + } else { + resultLog += `search succeeded: ${spawnResult.exitCode}` + } + getLogger().info(`CodeTransformation: searching for ${str} in ${project.path}, result = ${resultLog}`) + if (spawnResult.exitCode === 0) { + embeddedSQLProjects.push(project) + break + } + } + } + getLogger().info( + `CodeTransformation: found ${embeddedSQLProjects.length} projects with embedded SQL statements` + ) + telemetry.record({ + codeTransformMetadata: resultLog, + }) + }) + return embeddedSQLProjects } export async function setTransformationToRunningState() { diff --git a/packages/core/src/codewhisperer/models/constants.ts b/packages/core/src/codewhisperer/models/constants.ts index 3eab4d5eebf..d39771bc8ba 100644 --- a/packages/core/src/codewhisperer/models/constants.ts +++ b/packages/core/src/codewhisperer/models/constants.ts @@ -450,6 +450,10 @@ export const codeTransformLocThreshold = 100000 export const jobStartedChatMessage = 'I am starting to transform your code. It can take 10 to 30 minutes to upgrade your code, depending on the size of your project. To monitor progress, go to the Transformation Hub. If I run into any issues, I might pause the transformation to get input from you on how to proceed.' +export const chooseTransformationObjective = `I can help you with the following tasks:\n- Upgrade your Java 8 and Java 11 codebases to Java 17, or upgrade Java 17 code with up to date libraries and other dependencies.\n- Convert embedded SQL code for Oracle to PostgreSQL database migrations in AWS DMS.\n\nWhat would you like to do? You can enter "language upgrade" or "sql conversion".` + +export const chooseTransformationObjectivePlaceholder = 'Enter "language upgrade" or "sql conversion"' + export const uploadingCodeStepMessage = 'Upload your code' export const buildCodeStepMessage = 'Build uploaded code in secure build environment' @@ -477,6 +481,8 @@ export const failedStepMessage = 'The step failed, fetching additional details.. export const jobCompletedMessage = 'The transformation completed.' +export const noChangesMadeMessage = "I didn't make any changes for this transformation." + export const noOngoingJobMessage = 'No ongoing job.' export const nothingToShowMessage = 'Nothing to show' @@ -490,8 +496,7 @@ export const startTransformationButtonText = 'Start a new transformation' export const stopTransformationButtonText = 'Stop transformation' -export const checkingForProjectsChatMessage = - 'I am checking for open projects that are eligible for Code Transformation.' +export const checkingForProjectsChatMessage = 'Checking for eligible projects...' export const buildStartedChatMessage = 'I am building your project. This can take up to 10 minutes, depending on the size of your project.' @@ -507,7 +512,7 @@ export const absolutePathDetectedMessage = (numPaths: number, buildFile: string, export const unsupportedJavaVersionChatMessage = `I can only upgrade Java 8, Java 11, or Java 17 projects. For more information, see the [Amazon Q documentation](${codeTransformPrereqDoc}).` export const selectSQLMetadataFileHelpMessage = - 'Next, I need the zipped metadata file from your schema conversion. You can download the metadata by going to your migration project in the AWS DMS console. Open the schema conversion and choose **Convert the embedded SQL in your application**. You can downloaded the metadata from Amazon S3 in the {schema-conversion-project}/ directory.' + 'Okay, I can convert the embedded SQL code for your Oracle to PostgreSQL transformation. To get started, upload the zipped metadata file from your schema conversion in AWS Data Migration Service (DMS). To retrieve the metadata file:\n1. Open your database migration project in the AWS DMS console.\n2. Open the schema conversion and choose **Convert the embedded SQL in your application**.\n3. Choose the link to Amazon S3 console.\n\nYou can download the metadata file from the {schema-conversion-project}/ directory. For more info, refer to the [documentation](https://docs.aws.amazon.com/dms/latest/userguide/schema-conversion-save-apply.html#schema-conversion-save).' export const invalidMetadataFileUnsupportedSourceDB = 'I can only convert SQL for migrations from an Oracle source database. The provided .sct file indicates another source database for this migration.' @@ -578,19 +583,15 @@ export const jobCancelledChatMessage = export const jobCancelledNotification = 'You cancelled the transformation.' -export const jobCompletedChatMessage = - 'I upgraded your code. You can review the diff to see my proposed changes and accept or reject them. The transformation summary has details about the files I updated.' +export const jobCompletedChatMessage = `I transformed your code. You can review the diff to see my proposed changes and accept or reject them. The transformation summary has details about the files I updated.` -export const jobCompletedNotification = - 'Amazon Q upgraded your code. You can review the diff to see my proposed changes and accept or reject them. The transformation summary has details about the files I updated.' +export const jobCompletedNotification = `Amazon Q transformed your code. You can review the diff to see my proposed changes and accept or reject them. The transformation summary has details about the files I updated.` -export const jobPartiallyCompletedChatMessage = - 'I upgraded part of your code. You can review the diff to see my proposed changes and accept or reject them. The transformation summary has details about the files I updated and the errors that prevented a complete transformation.' +export const jobPartiallyCompletedChatMessage = `I transformed part of your code. You can review the diff to see my proposed changes and accept or reject them. The transformation summary has details about the files I updated and the errors that prevented a complete transformation.` -export const jobPartiallyCompletedNotification = - 'Amazon Q upgraded part of your code. You can review the diff to see my proposed changes and accept or reject them. The transformation summary has details about the files I updated and the errors that prevented a complete transformation.' +export const jobPartiallyCompletedNotification = `Amazon Q transformed part of your code. You can review the diff to see my proposed changes and accept or reject them. The transformation summary has details about the files I updated and the errors that prevented a complete transformation.` -export const noPomXmlFoundChatMessage = `I couldn\'t find a project that I can upgrade. Your Java project must be built on Maven and contain a pom.xml file. For more information, see the [Amazon Q documentation](${codeTransformPrereqDoc}).` +export const noPomXmlFoundChatMessage = `I couldn\'t find a project that I can upgrade. I couldn\'t find a pom.xml file in any of your open projects, nor could I find any embedded SQL statements. Currently, I can upgrade Java 8 or Java 11 projects built on Maven, or Oracle SQL to PostgreSQL statements in Java projects. For more information, see the [Amazon Q documentation](${codeTransformPrereqDoc}).` export const noPomXmlFoundNotification = `None of your open modules are supported for code transformation with Amazon Q. A pom.xml is required for transformation.` @@ -677,6 +678,10 @@ export const chooseSourceVersionFormTitle = 'Choose the source code version' export const chooseTargetVersionFormTitle = 'Choose the target code version' +export const chooseSchemaFormTitle = 'Choose the schema of the database' + +export const chooseProjectSchemaFormMessage = 'To continue, choose the project and schema for this transformation.' + export const skipUnitTestsFormTitle = 'Choose to skip unit tests' export const skipUnitTestsFormMessage = diff --git a/packages/core/src/codewhisperer/models/model.ts b/packages/core/src/codewhisperer/models/model.ts index f82ca81def4..b066a78ba0e 100644 --- a/packages/core/src/codewhisperer/models/model.ts +++ b/packages/core/src/codewhisperer/models/model.ts @@ -328,7 +328,7 @@ export class ZipManifest { buildLogs: string = 'build-logs.txt' version: string = '1.0' hilCapabilities: string[] = ['HIL_1pDependency_VersionUpgrade'] - transformCapabilities: string[] = ['EXPLAINABILITY_V1'] // TO-DO: for SQL conversions, maybe make this = [] + transformCapabilities: string[] = ['EXPLAINABILITY_V1'] customBuildCommand: string = 'clean test' requestedConversions?: { sqlConversion?: { diff --git a/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts b/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts index 94b7488fd2c..dc599a1dcd0 100644 --- a/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts +++ b/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts @@ -232,6 +232,8 @@ export async function uploadPayload(payloadFileName: string, uploadContext?: Upl */ const mavenExcludedExtensions = ['.repositories', '.sha1'] +const sourceExcludedExtensions = ['.DS_Store'] + /** * Determines if the specified file path corresponds to a Maven metadata file * by checking against known metadata file extensions. This is used to identify @@ -244,24 +246,22 @@ function isExcludedDependencyFile(path: string): boolean { return mavenExcludedExtensions.some((extension) => path.endsWith(extension)) } -/** - * Gets all files in dir. We use this method to get the source code, then we run a mvn command to - * copy over dependencies into their own folder, then we use this method again to get those - * dependencies. If isDependenciesFolder is true, then we are getting all the files - * of the dependencies which were copied over by the previously-run mvn command, in which case - * we DO want to include any dependencies that may happen to be named "target", hence the check - * in the first part of the IF statement. The point of excluding folders named target is that - * "target" is also the name of the folder where .class files, large JARs, etc. are stored after - * building, and we do not want these included in the ZIP so we exclude these when calling - * getFilesRecursively on the source code folder. - */ -function getFilesRecursively(dir: string, isDependenciesFolder: boolean): string[] { +// do not zip the .DS_Store file as it may appear in the diff.patch +function isExcludedSourceFile(path: string): boolean { + return sourceExcludedExtensions.some((extension) => path.endsWith(extension)) +} + +// zip all dependency files and all source files excluding "target" (contains large JARs) plus ".git" and ".idea" (may appear in diff.patch) +export function getFilesRecursively(dir: string, isDependenciesFolder: boolean): string[] { const entries = nodefs.readdirSync(dir, { withFileTypes: true }) const files = entries.flatMap((entry) => { const res = path.resolve(dir, entry.name) - // exclude 'target' directory from ZIP (except if zipping dependencies) due to issues in backend if (entry.isDirectory()) { - if (isDependenciesFolder || entry.name !== 'target') { + if (isDependenciesFolder) { + // include all dependency files + return getFilesRecursively(res, isDependenciesFolder) + } else if (entry.name !== 'target' && entry.name !== '.git' && entry.name !== '.idea') { + // exclude the above directories when zipping source code return getFilesRecursively(res, isDependenciesFolder) } else { return [] @@ -310,8 +310,8 @@ export async function zipCode( const sourceFiles = getFilesRecursively(projectPath, false) let sourceFilesSize = 0 for (const file of sourceFiles) { - if (nodefs.statSync(file).isDirectory()) { - getLogger().info('CodeTransformation: Skipping directory, likely a symlink') + if (nodefs.statSync(file).isDirectory() || isExcludedSourceFile(file)) { + getLogger().info('CodeTransformation: Skipping file') continue } const relativePath = path.relative(projectPath, file) @@ -334,16 +334,13 @@ export async function zipCode( target: transformByQState.getTargetDB(), schema: transformByQState.getSchema(), host: transformByQState.getSourceServerName(), - sctFileName: metadataZip.getEntries().filter((entry) => entry.entryName.endsWith('.sct'))[0] - .entryName, + sctFileName: metadataZip.getEntries().filter((entry) => entry.name.endsWith('.sct'))[0].name, }, } // TO-DO: later consider making this add to path.join(zipManifest.dependenciesRoot, 'qct-sct-metadata', entry.entryName) so that it's more organized metadataZip .getEntries() - .forEach((entry) => - zip.addFile(path.join(zipManifest.dependenciesRoot, entry.entryName), entry.getData()) - ) + .forEach((entry) => zip.addFile(path.join(zipManifest.dependenciesRoot, entry.name), entry.getData())) const sqlMetadataSize = (await nodefs.promises.stat(transformByQState.getMetadataPathSQL())).size getLogger().info(`CodeTransformation: SQL metadata file size = ${sqlMetadataSize}`) } diff --git a/packages/core/src/codewhisperer/service/transformByQ/transformationResultsViewProvider.ts b/packages/core/src/codewhisperer/service/transformByQ/transformationResultsViewProvider.ts index eab339df718..8bea3486bd3 100644 --- a/packages/core/src/codewhisperer/service/transformByQ/transformationResultsViewProvider.ts +++ b/packages/core/src/codewhisperer/service/transformByQ/transformationResultsViewProvider.ts @@ -145,6 +145,12 @@ export class DiffModel { */ public parseDiff(pathToDiff: string, pathToWorkspace: string): ProposedChangeNode[] { const diffContents = fs.readFileSync(pathToDiff, 'utf8') + + if (!diffContents.trim()) { + getLogger().error(`CodeTransformation: diff.patch file is empty`) + throw new Error(CodeWhispererConstants.noChangesMadeMessage) + } + const changedFiles = parsePatch(diffContents) // path to the directory containing copy of the changed files in the transformed project const pathToTmpSrcDir = this.copyProject(pathToWorkspace, changedFiles) diff --git a/packages/core/src/dev/config.ts b/packages/core/src/dev/config.ts index 276732a72bf..d5fa49b2426 100644 --- a/packages/core/src/dev/config.ts +++ b/packages/core/src/dev/config.ts @@ -10,6 +10,3 @@ export const betaUrl = { amazonq: '', toolkit: '', } - -// feature flag for SQL transformations -export const isSQLTransformReady = false diff --git a/packages/core/src/shared/utilities/workspaceUtils.ts b/packages/core/src/shared/utilities/workspaceUtils.ts index d8bd9177592..f9d56f5f4ea 100644 --- a/packages/core/src/shared/utilities/workspaceUtils.ts +++ b/packages/core/src/shared/utilities/workspaceUtils.ts @@ -17,6 +17,8 @@ import { sanitizeFilename } from './textUtilities' import { GitIgnoreAcceptor } from '@gerhobbelt/gitignore-parser' import * as parser from '@gerhobbelt/gitignore-parser' import fs from '../fs/fs' +import { ChildProcess } from './processUtils' +import { isWin } from '../vscode/env' type GitIgnoreRelativeAcceptor = { folderPath: string @@ -610,3 +612,24 @@ export async function collectFilesForIndex( // pick top 100k files below size limit return storage.slice(0, Math.min(100000, i)) } + +/** + * Performs a case-insensitive, recursive search for a string in a directory. + * note: + * 'error' is set when command fails to spawn; 'stderr' is set when command itself fails. + * 'exitCode' will be 0 when searchStr is detected (each line where it's found will be printed to 'stdout'). + * 'exitCode' will be non-zero and 'stdout' / 'stderr' / 'error' will all be empty/undefined when searchStr is not detected. + * @param searchStr the string to search for + * @param dirPath the path of the directory to search in + * @returns the result of the search + */ +export async function findStringInDirectory(searchStr: string, dirPath: string) { + const isWindows = isWin() + const command = isWindows ? 'findstr' : 'grep' + // case-insensitive, recursive search + const args = isWindows ? ['/i', '/s', searchStr, '*.*'] : ['-i', '-r', searchStr] + const spawnResult = await new ChildProcess(command, args).run({ + spawnOptions: { cwd: dirPath }, + }) + return spawnResult +} diff --git a/packages/core/src/test/codewhisperer/commands/transformByQ.test.ts b/packages/core/src/test/codewhisperer/commands/transformByQ.test.ts index 9aee508cf79..b294d07b586 100644 --- a/packages/core/src/test/codewhisperer/commands/transformByQ.test.ts +++ b/packages/core/src/test/codewhisperer/commands/transformByQ.test.ts @@ -32,6 +32,7 @@ import { updateJobHistory, zipCode, getTableMapping, + getFilesRecursively, } from '../../../codewhisperer/service/transformByQ/transformApiHandler' import { validateOpenProjects, @@ -288,6 +289,19 @@ describe('transformByQ', function () { }) }) + it(`WHEN getFilesRecursively on source code THEN ignores excluded directories`, async function () { + const sourceFolder = path.join(tempDir, 'src') + await fs.mkdir(sourceFolder) + await fs.writeFile(path.join(sourceFolder, 'HelloWorld.java'), 'sample content for the test file') + + const gitFolder = path.join(tempDir, '.git') + await fs.mkdir(gitFolder) + await fs.writeFile(path.join(gitFolder, 'config'), 'sample content for the test file') + + const zippedFiles = getFilesRecursively(tempDir, false) + assert.strictEqual(zippedFiles.length, 1) + }) + it(`WHEN getTableMapping on complete step 0 progressUpdates THEN map IDs to tables`, async function () { const stepZeroProgressUpdates = [ { diff --git a/packages/core/src/testInteg/shared/utilities/workspaceUtils.test.ts b/packages/core/src/testInteg/shared/utilities/workspaceUtils.test.ts index 73185735c01..e6bbb3bd83a 100644 --- a/packages/core/src/testInteg/shared/utilities/workspaceUtils.test.ts +++ b/packages/core/src/testInteg/shared/utilities/workspaceUtils.test.ts @@ -10,6 +10,7 @@ import { collectFiles, collectFilesForIndex, findParentProjectFile, + findStringInDirectory, getWorkspaceFoldersByPrefixes, getWorkspaceRelativePath, } from '../../../shared/utilities/workspaceUtils' @@ -509,4 +510,15 @@ describe('workspaceUtils', () => { ) }) }) + + describe('findStringInDirectory', function () { + it('prints the line with the detected string to stdout', async () => { + const fileAmount = 1 + const searchStr = 'oracle.jdbc.OracleDriver' + const fileContent = `test content ${searchStr} more test content` + const workspaceFolder = await createTestWorkspace(fileAmount, { fileContent: fileContent }) + const spawnResult = await findStringInDirectory(searchStr, workspaceFolder.uri.fsPath) + assert.equal(spawnResult.stdout.includes(searchStr), true) + }) + }) }) From 5eaffe49d5c82e4d994667664fa6d233f30a2134 Mon Sep 17 00:00:00 2001 From: Hweinstock <42325418+Hweinstock@users.noreply.github.com> Date: Mon, 18 Nov 2024 15:47:32 -0500 Subject: [PATCH 6/7] build(test): undo automatic retries. (#6045) ## Problem Some tests appear to be hanging indefinitely with the new retry system. Example: https://github.com/aws/aws-toolkit-vscode/actions/runs/11899590502/job/33158551534?pr=6037 ## Solution Rollback changes until proper solution is found. --- License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --- packages/core/src/test/testRunner.ts | 3 +-- packages/toolkit/test/unit/index.ts | 9 +++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/core/src/test/testRunner.ts b/packages/core/src/test/testRunner.ts index 9ba9c7cd01e..7a90977587c 100644 --- a/packages/core/src/test/testRunner.ts +++ b/packages/core/src/test/testRunner.ts @@ -22,7 +22,7 @@ export async function runTests( testFolder: string | string[], extensionId: string, initTests: string[] = [], - options?: { retries?: number; testFiles?: string[] } + options?: { testFiles?: string[] } ): Promise { if (!process.env['AWS_TOOLKIT_AUTOMATION']) { throw new Error('Expected the "AWS_TOOLKIT_AUTOMATION" environment variable to be set for tests.') @@ -79,7 +79,6 @@ export async function runTests( mochaFile: outputFile, }, }, - retries: options?.retries ?? 0, timeout: 0, }) diff --git a/packages/toolkit/test/unit/index.ts b/packages/toolkit/test/unit/index.ts index 6f2052d5647..998953c7fd4 100644 --- a/packages/toolkit/test/unit/index.ts +++ b/packages/toolkit/test/unit/index.ts @@ -7,10 +7,7 @@ import { runTests } from 'aws-core-vscode/test' import { VSCODE_EXTENSION_ID } from 'aws-core-vscode/utils' export function run(): Promise { - return runTests( - process.env.TEST_DIR ?? ['test/unit', '../../core/dist/src/test'], - VSCODE_EXTENSION_ID.awstoolkit, - ['../../core/dist/src/test/globalSetup.test.ts'], - { retries: 3 } - ) + return runTests(process.env.TEST_DIR ?? ['test/unit', '../../core/dist/src/test'], VSCODE_EXTENSION_ID.awstoolkit, [ + '../../core/dist/src/test/globalSetup.test.ts', + ]) } From f81d9a1f95142587fd8823da1025b0fc974f886c Mon Sep 17 00:00:00 2001 From: Will Lo <96078566+Will-ShaoHua@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:16:21 -0800 Subject: [PATCH 7/7] config(amazonq): update amazonq service api model (#6044) ## Problem update service model ## Solution --- License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --- .../codewhisperer/client/user-service-2.json | 46 +++++++++++++++++-- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/packages/core/src/codewhisperer/client/user-service-2.json b/packages/core/src/codewhisperer/client/user-service-2.json index c3233bc8b6c..bf20eeb6fa0 100644 --- a/packages/core/src/codewhisperer/client/user-service-2.json +++ b/packages/core/src/codewhisperer/client/user-service-2.json @@ -624,7 +624,9 @@ "acceptedCharacterCount": { "shape": "PrimitiveInteger" }, "totalCharacterCount": { "shape": "PrimitiveInteger" }, "timestamp": { "shape": "Timestamp" }, - "unmodifiedAcceptedCharacterCount": { "shape": "PrimitiveInteger" } + "unmodifiedAcceptedCharacterCount": { "shape": "PrimitiveInteger" }, + "totalNewCodeCharacterCount": { "shape": "PrimitiveInteger" }, + "totalNewCodeLineCount": { "shape": "PrimitiveInteger" } } }, "CodeFixAcceptanceEvent": { @@ -705,7 +707,19 @@ "codeScanJobId": { "shape": "CodeScanJobId" }, "timestamp": { "shape": "Timestamp" }, "codeAnalysisScope": { "shape": "CodeAnalysisScope" } - } + }, + "documentation": "

Published when a security scan or code review starts

" + }, + "CodeScanFailedEvent": { + "type": "structure", + "required": ["programmingLanguage", "codeScanJobId", "timestamp"], + "members": { + "programmingLanguage": { "shape": "ProgrammingLanguage" }, + "codeScanJobId": { "shape": "CodeScanJobId" }, + "timestamp": { "shape": "Timestamp" }, + "codeAnalysisScope": { "shape": "CodeAnalysisScope" } + }, + "documentation": "

Published when a security scan or code review fails

" }, "CodeScanJobId": { "type": "string", @@ -738,6 +752,18 @@ "documentation": "

Code Scan Remediations Interaction Type

", "enum": ["CODESCAN_ISSUE_HOVER", "CODESCAN_ISSUE_APPLY_FIX", "CODESCAN_ISSUE_VIEW_DETAILS"] }, + "CodeScanSucceededEvent": { + "type": "structure", + "required": ["programmingLanguage", "codeScanJobId", "timestamp", "numberOfFindings"], + "members": { + "programmingLanguage": { "shape": "ProgrammingLanguage" }, + "codeScanJobId": { "shape": "CodeScanJobId" }, + "timestamp": { "shape": "Timestamp" }, + "numberOfFindings": { "shape": "PrimitiveInteger" }, + "codeAnalysisScope": { "shape": "CodeAnalysisScope" } + }, + "documentation": "

Published when a security scan or code review completes successfully

" + }, "Completion": { "type": "structure", "required": ["content"], @@ -998,13 +1024,21 @@ "numberOfAddLines": { "shape": "PrimitiveInteger" }, "numberOfAddFiles": { "shape": "PrimitiveInteger" }, "userDecision": { "shape": "DocGenerationUserDecision" }, - "interactionType": { "shape": "DocGenerationInteractionType" } + "interactionType": { "shape": "DocGenerationInteractionType" }, + "userIdentity": { "shape": "String" }, + "numberOfNavigation": { "shape": "PrimitiveInteger" }, + "folderLevel": { "shape": "DocGenerationFolderLevel" } } }, + "DocGenerationFolderLevel": { + "type": "string", + "documentation": "

Doc Generation Folder Level

", + "enum": ["SUB_FOLDER", "ENTIRE_WORKSPACE"] + }, "DocGenerationInteractionType": { "type": "string", "documentation": "

Doc Generation Interaction Type

", - "enum": ["GENERATE_README", "UPDATE_README"] + "enum": ["GENERATE_README", "UPDATE_README", "EDIT_README"] }, "DocGenerationUserDecision": { "type": "string", @@ -2198,7 +2232,7 @@ }, "SuggestionState": { "type": "string", - "enum": ["ACCEPT", "REJECT", "DISCARD", "EMPTY"] + "enum": ["ACCEPT", "REJECT", "DISCARD", "EMPTY", "MERGE"] }, "SupplementalContext": { "type": "structure", @@ -2366,6 +2400,8 @@ "codeCoverageEvent": { "shape": "CodeCoverageEvent" }, "userModificationEvent": { "shape": "UserModificationEvent" }, "codeScanEvent": { "shape": "CodeScanEvent" }, + "codeScanSucceededEvent": { "shape": "CodeScanSucceededEvent" }, + "codeScanFailedEvent": { "shape": "CodeScanFailedEvent" }, "codeScanRemediationsEvent": { "shape": "CodeScanRemediationsEvent" }, "codeFixGenerationEvent": { "shape": "CodeFixGenerationEvent" }, "codeFixAcceptanceEvent": { "shape": "CodeFixAcceptanceEvent" },